llvm dev - Apr 2021 - [RFC] [X86] Emit unaligned vector moves on avx machine with option control.

>
>
> I understand your goal is to find and fix bugs in software that is
> still under development and CAN be fixed.  I fully endorse that
> goal.  However, that is not the situation that Sony has, and likely
> not what Intel has.  Your proposal will NOT solve our problem.

No, that's not it at all! I'm afraid you've totally misunderstood my
concern.

My goal is that if we add a compiler feature to address this problem -- so
that you can compile code with under-aligned objects, and have it work as
the author expected it to --  that the feature *reliably *addresses the
problem, and makes such code no longer exhibit Undefined Behavior. The
proposed backend change does not accomplish that, but we can implement a
feature which will.

As Reid said, -fmax-type-align=N appears to be *almost* that feature,
and something like this little patch (along with documentation update) may
be all that's needed (but this is totally untested).

diff --git clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenModule.cpp
index b23d995683bf..3aef166a690e 100644
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6280,8 +6280,7 @@ CharUnits
CodeGenModule::getNaturalTypeAlignment(QualType T,
   // Cap to the global maximum type alignment unless the alignment
   // was somehow explicit on the type.
   if (unsigned MaxAlign = getLangOpts().MaxTypeAlign) {
-    if (Alignment.getQuantity() > MaxAlign &&
-        !getContext().isAlignmentRequired(T))
+    if (Alignment.getQuantity() > MaxAlign)
       Alignment = CharUnits::fromQuantity(MaxAlign);
   }
   return Alignment;
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210419/e8532c19/attachment-0001.html>

(And while I did initially also express skepticism about whether this
feature is necessary at all -- even if properly implemented -- I think the
fact that Intel, Apple, and Sony have all proposed or implemented something
along these lines is good evidence that it is sufficiently useful. I'm
convinced on that point!)

On Mon, Apr 19, 2021 at 2:30 PM James Y Knight <jyknight at google.com>
wrote:
>
>> I understand your goal is to find and fix bugs in software that is
>> still under development and CAN be fixed.  I fully endorse that
>> goal.  However, that is not the situation that Sony has, and likely
>> not what Intel has.  Your proposal will NOT solve our problem.
>
>
> No, that's not it at all! I'm afraid you've totally
misunderstood my
> concern.
>
> My goal is that if we add a compiler feature to address this problem -- so
> that you can compile code with under-aligned objects, and have it work as
> the author expected it to --  that the feature *reliably *addresses the
> problem, and makes such code no longer exhibit Undefined Behavior. The
> proposed backend change does not accomplish that, but we can implement a
> feature which will.
>
> As Reid said, -fmax-type-align=N appears to be *almost* that feature,
> and something like this little patch (along with documentation update) may
> be all that's needed (but this is totally untested).
>
> diff --git clang/lib/CodeGen/CodeGenModule.cpp
> clang/lib/CodeGen/CodeGenModule.cpp
> index b23d995683bf..3aef166a690e 100644
> --- clang/lib/CodeGen/CodeGenModule.cpp
> +++ clang/lib/CodeGen/CodeGenModule.cpp
> @@ -6280,8 +6280,7 @@ CharUnits
> CodeGenModule::getNaturalTypeAlignment(QualType T,
>    // Cap to the global maximum type alignment unless the alignment
>    // was somehow explicit on the type.
>    if (unsigned MaxAlign = getLangOpts().MaxTypeAlign) {
> -    if (Alignment.getQuantity() > MaxAlign &&
> -        !getContext().isAlignmentRequired(T))
> +    if (Alignment.getQuantity() > MaxAlign)
>        Alignment = CharUnits::fromQuantity(MaxAlign);
>    }
>    return Alignment;
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210419/6912659a/attachment.html>

We might still not be fully understanding one another, because this:
so that you can compile code with under-aligned objects, and have it work as the
author expected it to
sounds like you’re expecting us to recompile the client code that creates the
under-aligned objects.  That is literally not possible.  If you do understand
that part, great, it’s just not obvious to me from how you’re phrasing things.

I (still) don’t know what Intel is facing.  For Sony’s problem, we would be much
more likely to try to do something specific to the APIs that are being abused,
rather than something draconian like eliminating alignment requirements for
everyone.  But of course we have a solution that works for us, so there’s that
much more inertia to overcome.
--paulr

From: James Y Knight <jyknight at google.com>
Sent: Monday, April 19, 2021 2:30 PM
To: Robinson, Paul <paul.robinson at sony.com>
Cc: Luo, Yuanke <yuanke.luo at intel.com>; Roman Lebedev <lebedev.ri at
gmail.com>; Liu, Chen3 <chen3.liu at intel.com>; llvm-dev <llvm-dev
at lists.llvm.org>; Maslov, Sergey V <sergey.v.maslov at intel.com>;
daniel.towner at intel.com
Subject: Re: [llvm-dev] [RFC] [X86] Emit unaligned vector moves on avx machine
with option control.


I understand your goal is to find and fix bugs in software that is
still under development and CAN be fixed.  I fully endorse that
goal.  However, that is not the situation that Sony has, and likely
not what Intel has.  Your proposal will NOT solve our problem.

No, that's not it at all! I'm afraid you've totally misunderstood my
concern.

My goal is that if we add a compiler feature to address this problem -- so that
you can compile code with under-aligned objects, and have it work as the author
expected it to --  that the feature reliably addresses the problem, and makes
such code no longer exhibit Undefined Behavior. The proposed backend change does
not accomplish that, but we can implement a feature which will.

As Reid said, -fmax-type-align=N appears to be almost that feature, and
something like this little patch (along with documentation update) may be all
that's needed (but this is totally untested).

diff --git clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenModule.cpp
index b23d995683bf..3aef166a690e 100644
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6280,8 +6280,7 @@ CharUnits CodeGenModule::getNaturalTypeAlignment(QualType
T,
   // Cap to the global maximum type alignment unless the alignment
   // was somehow explicit on the type.
   if (unsigned MaxAlign = getLangOpts().MaxTypeAlign) {
-    if (Alignment.getQuantity() > MaxAlign &&
-        !getContext().isAlignmentRequired(T))
+    if (Alignment.getQuantity() > MaxAlign)
       Alignment = CharUnits::fromQuantity(MaxAlign);
   }
   return Alignment;

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210419/8f702fe1/attachment.html>