[PATCH] D46379: [ConstantFold] Turn off expression simplification for vector type

[Karl-Johan Karlsson via Phabricator via llvm-commits]

Ka-Ka updated this revision to Diff 149433.
Ka-Ka added a comment.

In https://reviews.llvm.org/D46379#1118168, @spatel wrote:

> This is a constant folding change, so the test should be visible with just "opt -constprop ..."
>
> Ie, the test should be minimized and moved under test/Analysis/ConstantFolding.


I agree, it is much better with a smaller testcase.  It turned out to be much easier than I initially thought to create a testcase with "opt -constprop ..."

> I don't know this code. Is the bug only related to GEP? 
>  But this patch also affects cast-of-cast when the outer/second cast is to a vector type? Eg, you don't want to inhibit this:
> 
>   define <4 x i16> @test_mmx_const() {
>     %A = bitcast <2 x i32> zeroinitializer to x86_mmx
>     %B = bitcast x86_mmx %A to <4 x i16>
>     ret <4 x i16> %B
>   }
>   
> 
> 
> 
> 
>   $ opt -constprop foo.ll -S
>   define <4 x i16> @test_mmx_const() {
>     ret <4 x i16> zeroinitializer
>   }
>   
> 
> 
> (so there should be a negative test for that example in this patch)

You are correct, after reading the code again I'm now convinced that the fault is only related to GEP. I have updated the patch to only affect the GEP case.

Do you think I should still include the negative testcase that you specified above even if the patch now only affect the GEP case?


Repository:
  rL LLVM

https://reviews.llvm.org/D46379

Files:
  lib/IR/ConstantFold.cpp
  test/Analysis/ConstantFolding/gep-zeroinit-vector.ll
  test/Transforms/LoopVectorize/X86/constant-fold.ll


Index: test/Transforms/LoopVectorize/X86/constant-fold.ll
===================================================================
--- test/Transforms/LoopVectorize/X86/constant-fold.ll
+++ test/Transforms/LoopVectorize/X86/constant-fold.ll
@@ -2,7 +2,6 @@
 ; RUN: opt -loop-vectorize -S -mtriple=x86_64-- -o - %s | FileCheck %s
 
 ; Testcase that point out faulty bitcast that cast between different sizes.
-; See "bitcast ([1 x %rec8]* @a to <2 x i16*>)" in checks below
 
 %rec8 = type { i16 }
 
@@ -28,7 +27,7 @@
 ; CHECK-NEXT:    [[TMP3:%.*]] = getelementptr [2 x i16*], [2 x i16*]* @b, i16 0, i64 [[TMP2]]
 ; CHECK-NEXT:    [[TMP4:%.*]] = getelementptr i16*, i16** [[TMP3]], i32 0
 ; CHECK-NEXT:    [[TMP5:%.*]] = bitcast i16** [[TMP4]] to <2 x i16*>*
-; CHECK-NEXT:    store <2 x i16*> bitcast ([1 x %rec8]* @a to <2 x i16*>), <2 x i16*>* [[TMP5]], align 8
+; CHECK-NEXT:    store <2 x i16*> <i16* getelementptr inbounds (%rec8, %rec8* extractelement (<2 x %rec8*> getelementptr ([1 x %rec8], [1 x %rec8]* @a, <2 x i16> zeroinitializer, <2 x i64> zeroinitializer), i32 0), i32 0, i32 0), i16* getelementptr inbounds (%rec8, %rec8* extractelement (<2 x %rec8*> getelementptr ([1 x %rec8], [1 x %rec8]* @a, <2 x i16> zeroinitializer, <2 x i64> zeroinitializer), i32 1), i32 0, i32 0)>, <2 x i16*>* [[TMP5]], align 8
 ; CHECK-NEXT:    [[INDEX_NEXT]] = add i32 [[INDEX]], 2
 ; CHECK-NEXT:    [[TMP6:%.*]] = icmp eq i32 [[INDEX_NEXT]], 2
 ; CHECK-NEXT:    br i1 [[TMP6]], label [[MIDDLE_BLOCK:%.*]], label [[VECTOR_BODY]], !llvm.loop !0
Index: test/Analysis/ConstantFolding/gep-zeroinit-vector.ll
===================================================================
--- /dev/null
+++ test/Analysis/ConstantFolding/gep-zeroinit-vector.ll
@@ -0,0 +1,14 @@
+; RUN: opt -constprop %s -S -o - | FileCheck %s
+
+%rec8 = type { i16 }
+ at a = global [1 x %rec8] zeroinitializer
+
+define <2 x i16*> @test_gep() {
+  %A = getelementptr [1 x %rec8], [1 x %rec8]* @a, <2 x i16> zeroinitializer, <2 x i64> zeroinitializer
+  %B = bitcast <2 x %rec8*> %A to <2 x i16*>
+  ret <2 x i16*> %B
+}
+
+; CHECK-LABEL: @test_gep
+; CHECK-NOT: ret <2 x i16*> bitcast ([1 x %rec8]* @a to <2 x i16*>)
+; CHECK-NEXT: ret <2 x i16*> <i16* getelementptr inbounds (%rec8, %rec8* extractelement (<2 x %rec8*> getelementptr ([1 x %rec8], [1 x %rec8]* @a, <2 x i64> zeroinitializer, <2 x i64> zeroinitializer), i32 0), i32 0, i32 0), i16* getelementptr inbounds (%rec8, %rec8* extractelement (<2 x %rec8*> getelementptr ([1 x %rec8], [1 x %rec8]* @a, <2 x i64> zeroinitializer, <2 x i64> zeroinitializer), i32 1), i32 0, i32 0)>
Index: lib/IR/ConstantFold.cpp
===================================================================
--- lib/IR/ConstantFold.cpp
+++ lib/IR/ConstantFold.cpp
@@ -545,7 +545,11 @@
                opc != Instruction::AddrSpaceCast &&
                // Do not fold bitcast (gep) with inrange index, as this loses
                // information.
-               !cast<GEPOperator>(CE)->getInRangeIndex().hasValue()) {
+               !cast<GEPOperator>(CE)->getInRangeIndex().hasValue() &&
+               // Do not fold if the gep type is a vector, as bitcasting
+               // operand 0 of a vector gep will result in a bitcast between
+               // different sizes.
+               !CE->getType()->isVectorTy()) {
       // If all of the indexes in the GEP are null values, there is no pointer
       // adjustment going on.  We might as well cast the source pointer.
       bool isAllNull = true;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46379.149433.patch
Type: text/x-patch
Size: 3496 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180601/853c002c/attachment-0001.bin>