[PATCH] Make DataLayout Non-Optional in the Module

Eric Christopher echristo at gmail.com
Tue Mar 3 15:06:36 PST 2015


In general it's looking pretty good. Couple more comments inline and one other which is "You pass a datalayout on the command line in some testcases and put it in the module in others. Any reason for the disparity?"

Thanks!

-eric


================
Comment at: test/Transforms/InstCombine/overflow-mul.ll:180
@@ +179,3 @@
+
+; Need this weird datalayout to keep the "and i32 %mul, 255" together
+target datalayout = "i32:8:8"
----------------
Can't put this at the top? Also, might want to explain why.

================
Comment at: test/Transforms/InstCombine/store.ll:44
@@ -43,3 +43,3 @@
 ; CHECK: Cont:
-; CHECK-NEXT:  %storemerge = phi i32 [ 47, %Cond2 ], [ -987654321, %Cond ]
+; CHECK-NEXT:  %storemerge = phi i32 [ -987654321, %Cond ], [ 47, %Cond2 ]
 ; CHECK-NEXT:  ret i32 %storemerge
----------------
Inquiring minds here on the swap?

================
Comment at: test/Transforms/LoopStrengthReduce/X86/2011-07-20-DoubleIV.ll:7
@@ -6,1 +6,3 @@
 
+; Needs to provide legal integer type in the DataLayout
+target datalayout = "n8:16:32:64"
----------------
This comment (repeated in a number of testcases) is a little oddly worded. Perhaps:

"Provide legal integer types."?

-eric

http://reviews.llvm.org/D7992

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list