[Bug tree-optimization/91758] New: Clang fails to pass validation after r261089

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/91758] New: Clang fails to pass validation after r261089

rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91758

            Bug ID: 91758
           Summary: Clang fails to pass validation after r261089
           Product: gcc
           Version: 9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: lukebenes at hotmail dot com
  Target Milestone: ---

With gcc 9 or newer clang will build but fail up to 150 validation tests.

The failures first started after:
https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=261089
     gimple-ssa-store-merging.c: Include gimple-fold.h.

While subsequent gcc tweaks resulted in different tests failing, this commit
appears to be the genesis of all of the failures.

Steps to Reproduce:
1. git clone https://github.com/llvm/llvm-project.git
2. cd llvm-project
3. mkdir build && cd build
4. CC=gcc CXX=g++ cmake -DLLVM_ENABLE_PROJECTS=clang -DCMAKE_BUILD_TYPE=Release
-G "Unix Makefiles" ../llvm
5. make check-all -j4

Expected Results:
All tests pass

Actual Results:
4-150 tests will fails such as
    Clang :: CodeGen/arm-fp16-arguments.c
    Clang :: CodeGen/lanai-regparm.c
    Clang :: CodeGen/ppc64-complex-parms.c
    Clang :: CodeGen/ppc64-complex-return.c
    Clang :: CodeGen/ppc64-vector.c
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/91758] Clang fails to pass validation after r261089

rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91758

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2019-09-12
                 CC|                            |ebotcazou at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #1 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---

> The failures first started after:
> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=261089
>      gimple-ssa-store-merging.c: Include gimple-fold.h.
>
> While subsequent gcc tweaks resulted in different tests failing, this commit
> appears to be the genesis of all of the failures.
>
> Steps to Reproduce:
> 1. git clone https://github.com/llvm/llvm-project.git
> 2. cd llvm-project
> 3. mkdir build && cd build
> 4. CC=gcc CXX=g++ cmake -DLLVM_ENABLE_PROJECTS=clang
> -DCMAKE_BUILD_TYPE=Release -G "Unix Makefiles" ../llvm
> 5. make check-all -j4

Thanks for reporting the problem, but we need more information and a
reproducer, see the instructions at https://gcc.gnu.org/bugs/
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/91758] Clang fails to pass validation after r261089

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91758

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |ASSIGNED
                 CC|                            |marxin at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |marxin at gcc dot gnu.org

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
I've reduced that and I'm working on that right now.
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/91758] Clang fails to pass validation after r261089

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91758

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
So that's what I have:
1) reduced LLVM test-case:

$ cat /tmp/llvm-project/clang/test/CodeGen/lanai-regparm.c
void f2(int a) __attribute((regparm(0)));
void f0() {
  f2(1);
}

2) I applied the following local patch to make the change smaller:

diff --git a/clang/include/clang/CodeGen/CGFunctionInfo.h
b/clang/include/clang/CodeGen/CGFunctionInfo.h
index 1f81072e23d..ef7f7410c9b 100644
--- a/clang/include/clang/CodeGen/CGFunctionInfo.h
+++ b/clang/include/clang/CodeGen/CGFunctionInfo.h
@@ -87,14 +87,14 @@ private:
     unsigned AllocaFieldIndex; // isInAlloca()
   };
   Kind TheKind;
-  bool PaddingInReg : 1;
-  bool InAllocaSRet : 1;    // isInAlloca()
-  bool IndirectByVal : 1;   // isIndirect()
-  bool IndirectRealign : 1; // isIndirect()
-  bool SRetAfterThis : 1;   // isIndirect()
+  bool PaddingInReg;
+  bool InAllocaSRet;    // isInAlloca()
+  bool IndirectByVal;   // isIndirect()
+  bool CanBeFlattened;   // isDirect()
+  bool SignExt ;         // isExtend()
+  bool SRetAfterThis ;   // isIndirect()
+  bool IndirectRealign: 1; // isIndirect()
   bool InReg : 1;           // isDirect() || isExtend() || isIndirect()
-  bool CanBeFlattened: 1;   // isDirect()
-  bool SignExt : 1;         // isExtend()

   bool canHavePaddingType() const {
     return isDirect() || isExtend() || isIndirect() || isExpand();
diff --git a/clang/lib/CodeGen/TargetInfo.cpp
b/clang/lib/CodeGen/TargetInfo.cpp
index f2696a33cfb..b115d9da145 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -7567,7 +7567,10 @@ public:
     if (!getCXXABI().classifyReturnType(FI))
       FI.getReturnInfo() = classifyReturnType(FI.getReturnType());
     for (auto &I : FI.arguments())
+    {
       I.info = classifyArgumentType(I.type, State);
+      __builtin_printf ("after: I.info: %d\n", I.info.getInReg ());
+    }
   }

   ABIArgInfo getIndirectResult(QualType Ty, bool ByVal, CCState &State) const;
@@ -7658,7 +7661,9 @@ ABIArgInfo LanaiABIInfo::classifyArgumentType(QualType
Ty,
   }
   if (InReg)
     return ABIArgInfo::getDirectInReg();
-  return ABIArgInfo::getDirect();
+  ABIArgInfo ret = ABIArgInfo::getDirect();
+  __builtin_printf ("before: %d\n", ret.getInReg ());
+  return ret;
 }

 namespace {

3) I see the problematic file is:
/tmp/llvm-project/clang/lib/CodeGen/TargetInfo.cpp
4) I took the patch from r261089 and applied it to r255894 and it still fails
5) apparently one needs -O3 to expose the issue
6) If I add following dbg_cnt:

diff --git a/gcc/dbgcnt.def b/gcc/dbgcnt.def
index 0421fae7bc0..3830666dc6c 100644
--- a/gcc/dbgcnt.def
+++ b/gcc/dbgcnt.def
@@ -195,3 +195,4 @@ DEBUG_COUNTER (tree_sra)
 DEBUG_COUNTER (vect_loop)
 DEBUG_COUNTER (vect_slp)
 DEBUG_COUNTER (dom_unreachable_edges)
+DEBUG_COUNTER (store_merging)
diff --git a/gcc/gimple-ssa-store-merging.c b/gcc/gimple-ssa-store-merging.c
index 3c63e75fcf6..2369fd4bf5d 100644
--- a/gcc/gimple-ssa-store-merging.c
+++ b/gcc/gimple-ssa-store-merging.c
@@ -166,6 +166,7 @@
 #include "rtl.h"
 #include "expr.h"      /* For get_bit_range.  */
 #include "optabs-tree.h"
+#include "dbgcnt.h"
 #include "selftest.h"

 /* The maximum size (in bits) of the stores this pass should generate.  */
@@ -3898,7 +3899,8 @@ imm_store_chain_info::output_merged_stores ()
   bool ret = false;
   FOR_EACH_VEC_ELT (m_merged_store_groups, i, merged_store)
     {
-      if (output_merged_store (merged_store))
+      if (dbg_cnt (store_merging)
+         && output_merged_store (merged_store))
        {
          unsigned int j;
          store_immediate_info *store;

I can bisect that to one store merging transformation:

before:
  MEM[(struct SmallVectorBase *)&Elements].Size = 0;
  MEM[(struct SmallVectorBase *)&Elements].Capacity = 3;

after:
  MEM[(unsigned int *)&Elements + 8B] = 12884901888;
  if (SizeInRegs_144 > 3)

The transformation looks fine to me and it must be an issue in RTL, because
this is the only difference I see in tree optimized dump file.

The change happens in {anonymous}::ARCABIInfo::computeInfo (const struct
ARCABIInfo * const this, struct CGFunctionInfo & FI)
function. And the valgrind also points to the same function:

valgrind /tmp/llvm-project/build/bin/clang -cc1  -triple lanai-unknown-unknown
-mregparm 4 /tmp/llvm-project/clang/test/CodeGen/lanai-regparm.c -emit-llvm
==766== Memcheck, a memory error detector
==766== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==766== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==766== Command: /tmp/llvm-project/build/bin/clang -cc1 -triple
lanai-unknown-unknown -mregparm 4
/tmp/llvm-project/clang/test/CodeGen/lanai-regparm.c -emit-llvm
==766==
before: 0
==766== Conditional jump or move depends on uninitialised value(s)
==766==    at 0x7295287: __vfprintf_internal (vfprintf-internal.c:1644)
==766==    by 0x7280C7A: printf (printf.c:33)
==766==    by 0x2776A28: (anonymous
namespace)::LanaiABIInfo::computeInfo(clang::CodeGen::CGFunctionInfo&) const
(in /tmp/llvm-project/build/bin/clang-10)
==766==    by 0x27F7533:
clang::CodeGen::CodeGenTypes::arrangeLLVMFunctionInfo(clang::CanQual<clang::Type>,
bool, bool, llvm::ArrayRef<clang::CanQual<clang::Type> >,
clang::FunctionType::ExtInfo,
llvm::ArrayRef<clang::FunctionType::ExtParameterInfo>,
clang::CodeGen::RequiredArgs) (in /tmp/llvm-project/build/bin/clang-10)
==766==    by 0x27F9D94:
clang::CodeGen::CodeGenTypes::arrangeFreeFunctionType(clang::CanQual<clang::FunctionProtoType>)
(in /tmp/llvm-project/build/bin/clang-10)
==766==    by 0x272AAC9:
clang::CodeGen::CodeGenTypes::ConvertFunctionTypeInternal(clang::QualType) (in
/tmp/llvm-project/build/bin/clang-10)
==766==    by 0x272BD45:
clang::CodeGen::CodeGenTypes::ConvertType(clang::QualType) (in
/tmp/llvm-project/build/bin/clang-10)
==766==    by 0x2701D5A:
clang::CodeGen::CodeGenModule::GetAddrOfFunction(clang::GlobalDecl,
llvm::Type*, bool, bool, clang::CodeGen::ForDefinition_t) (in
/tmp/llvm-project/build/bin/clang-10)
==766==    by 0x28472E2:
EmitFunctionDeclPointer(clang::CodeGen::CodeGenModule&, clang::FunctionDecl
const*) (in /tmp/llvm-project/build/bin/clang-10)
==766==    by 0x2861F2C:
clang::CodeGen::CodeGenFunction::EmitCallee(clang::Expr const*) (in
/tmp/llvm-project/build/bin/clang-10)
==766==    by 0x28621F2:
clang::CodeGen::CodeGenFunction::EmitCallExpr(clang::CallExpr const*,
clang::CodeGen::ReturnValueSlot) (in /tmp/llvm-project/build/bin/clang-10)
==766==    by 0x2897478: (anonymous
namespace)::ScalarExprEmitter::VisitCallExpr(clang::CallExpr const*) (in
/tmp/llvm-project/build/bin/clang-10)

7) The wrong output is then:
marxin@marxinbox:/tmp/llvm-project/build> /tmp/llvm-project/build/bin/clang
-cc1  -triple lanai-unknown-unknown -mregparm 4
/tmp/llvm-project/clang/test/CodeGen/lanai-regparm.c -emit-llvm
before: 0
after: I.info: 1
marxin@marxinbox:/tmp/llvm-project/build> /tmp/llvm-project/build/bin/clang
-cc1  -triple lanai-unknown-unknown -mregparm 4
/tmp/llvm-project/clang/test/CodeGen/lanai-regparm.c -emit-llvm
before: 0
after: I.info: 0

So I.info.getInReg() is sometimes 0 and sometimes 1.
I'm going to carry on ..
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/91758] Clang fails to pass validation after r261089

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91758

--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
So no, the affective change is:

  D.1259546.IndirectRealign = SR.6335_125;
  D.1259546.InReg = SR.6336_130;

  _163 = (unsigned char) SR.6335_125;
  _50 = (unsigned char) SR.6336_130;
  _140 = _50 << 1;
  _34 = _140 | _163;
  _383 = MEM[(struct ABIArgInfo *)&D.1259546 + 27B];
  _382 = _383 & 252;
  _358 = _34 & 3;
  _381 = _382 | _358;
  MEM[(struct ABIArgInfo *)&D.1259546 + 27B] = _381;

which is also fine from store-merging point of view.
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/91758] Clang fails to pass validation after r261089

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91758

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
Ok, so without changed source file I see optimized dump changed from:

  D.1259546.PaddingInReg = SR.6329_147;
  D.1259546.IndirectByVal = SR.6330_121;
  D.1259546.IndirectRealign = SR.6331_117;
  D.1259546.SRetAfterThis = SR.6332_29;
  D.1259546.InReg = SR.6333_124;
  D.1259546.CanBeFlattened = SR.6334_129;
  MEM[base: __for_begin_145, offset: 8B] = MEM[(struct ABIArgInfo
*)&D.1259546];

into:

  _139 = (unsigned char) SR.6329_147;
  _49 = (unsigned char) SR.6330_121;
  _137 = _49 << 2;
  _34 = _137 | _139;
  _382 = (unsigned char) SR.6331_117;
  _381 = _382 << 3;
  _357 = _381 | _34;
  _380 = (unsigned char) SR.6332_29;
  _377 = _380 << 4;
  _376 = _377 | _357;
  _375 = (unsigned char) SR.6333_124;
  _397 = _375 << 5;
  _401 = _397 | _376;
  _181 = (unsigned char) SR.6334_129;
  _364 = _181 << 6;
  _118 = _364 | _401;
  _131 = MEM[(struct ABIArgInfo *)&D.1259546 + 21B];
  _363 = _131 & 130;
  _366 = _118 & 125;
  _69 = _363 | _366;
  MEM[(struct ABIArgInfo *)&D.1259546 + 21B] = _69;
  MEM[base: __for_begin_145, offset: 8B] = MEM[(struct ABIArgInfo
*)&D.1259546];

which eventually ends up with something like:

        addq    $32, %rbx       #, ivtmp.6357
        movzbl  43(%rsp), %ecx  # %sfp, SR.6331
        movq    24(%rsp), %xmm0 # %sfp, tmp336
        movl    %r15d, -8(%rbx) # SR.6327, MEM[base: __for_begin_145, offset:
8B]
        leal    0(,%rdi,4), %edx        #, tmp337
# /tmp/llvm-project/clang/lib/CodeGen/TargetInfo.cpp:7572:      
__builtin_printf ("after: I.info: %d\n", I.info.getInReg ());
        leaq    .LC156(%rip), %rdi      #,
# /tmp/llvm-project/clang/lib/CodeGen/TargetInfo.cpp:7571:       I.info =
classifyArgumentType(I.type, State);
        movl    %r15d, 80(%rsp) # SR.6327, MEM[(struct ABIArgInfo *)&D.1259546
+ 16B]
        sall    $3, %ecx        #, tmp339
        orl     %r14d, %edx     # SR.6329, tmp338
        movhps  8(%rsp), %xmm0  # %sfp, tmp336
        orl     %ecx, %edx      # tmp339, tmp340
        movzbl  42(%rsp), %ecx  # %sfp, tmp341
        movups  %xmm0, -24(%rbx)        # tmp336, MEM[base: __for_begin_145,
offset: 8B]
        movaps  %xmm0, 64(%rsp) # tmp336, MEM[(struct ABIArgInfo *)&D.1259546]
        sall    $4, %ecx        #, tmp341
        orl     %ecx, %edx      # tmp341, tmp342
        orl     %eax, %edx      # tmp343, tmp344
        movl    %ebp, %eax      # SR.6334, tmp345
        sall    $6, %eax        #, tmp345
        orl     %eax, %edx      # tmp345, tmp346
        movzbl  85(%rsp), %eax  # MEM[(struct ABIArgInfo *)&D.1259546 + 21B],
tmp348
        andl    $125, %edx      #, tmp347
        andl    $-126, %eax     #, tmp348
        orl     %eax, %edx      # tmp348, tmp350
# /tmp/llvm-project/clang/lib/CodeGen/TargetInfo.cpp:7572:      
__builtin_printf ("after: I.info: %d\n", I.info.getInReg ());
        xorl    %eax, %eax      #
# /tmp/llvm-project/clang/lib/CodeGen/TargetInfo.cpp:7571:       I.info =
classifyArgumentType(I.type, State);
        movb    %dl, 85(%rsp)   # tmp350, MEM[(struct ABIArgInfo *)&D.1259546 +
21B]

and the problem is that we load:
0x2776490 <(anonymous
namespace)::LanaiABIInfo::computeInfo(clang::CodeGen::CGFunctionInfo&)
const+400> movzbl 0x2b(%rsp),%ecx

with the following value:
(gdb) p /t $ecx
$1 = 11110111

and then we or (11110111 << 3) into %edx and so that we end up with the 6th bit
set.
That is the InReg field.

Looking at the corresponding ASM code without the store-merging, there are
quite some
        andl    $1, %esi        #, tmp329
...
        andl    $1, %ecx        #, tmp347
and so on.

@Eric, @Jakub, @Richard: Aren't we missing something similar with the store
merging of bool:1 bit fields?
I can see the cast to 'unsigned char' from 'bool' in GIMPLE. Both should be 1B
and
so that we maybe encorporate random bits?
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/91758] Clang fails to pass validation after r261089

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91758

--- Comment #6 from Martin Liška <marxin at gcc dot gnu.org> ---
> @Eric, @Jakub, @Richard: Aren't we missing something similar with the store
> merging of bool:1 bit fields?
> I can see the cast to 'unsigned char' from 'bool' in GIMPLE. Both should be
> 1B and
> so that we maybe encorporate random bits?

I made a wrong assumption, one can't have value of boolean type different from
0/1. Otherwise, it will be an UBSAN:

snippet2.c:34:18: runtime error: load of value 255, which is not a valid value
for type 'bool'
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/91758] [9 Regression] Clang fails to pass validation after r261089

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91758

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |INVALID

--- Comment #7 from Martin Liška <marxin at gcc dot gnu.org> ---
I've got it. It's a usage of an uninitialize member variable in a class:
https://bugs.llvm.org/show_bug.cgi?id=40547#c38
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/91758] Clang fails to pass validation after r261089

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91758

--- Comment #8 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> I've got it. It's a usage of an uninitialize member variable in a class:
> https://bugs.llvm.org/show_bug.cgi?id=40547#c38

Thanks for catching this!
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/91758] Clang fails to pass validation after r261089

rguenth at gcc dot gnu.org
In reply to this post by rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91758

--- Comment #9 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Eric Botcazou from comment #8)
> > I've got it. It's a usage of an uninitialize member variable in a class:
> > https://bugs.llvm.org/show_bug.cgi?id=40547#c38
>
> Thanks for catching this!

You're welcome. It took me almost the whole day yesterday, but I learn more
about store merging (and read quite some RTL dumps).