[Attributor][FIX] Properly promote arguments pointers to arrays
authorJohannes Doerfert <johannes@jdoerfert.de>
Thu, 29 Oct 2020 05:13:27 +0000 (00:13 -0500)
committerJohannes Doerfert <johannes@jdoerfert.de>
Thu, 29 Oct 2020 05:45:32 +0000 (00:45 -0500)
When we promote pointer arguments we did compute a wrong offset and use
a wrong type for the array case.

Bug reported and reduced by Whitney Tsang <whitneyt@ca.ibm.com>.

llvm/lib/Transforms/IPO/AttributorAttributes.cpp
llvm/test/Transforms/Attributor/ArgumentPromotion/array.ll [new file with mode: 0644]

index bdaad78..4f08bd2 100644 (file)
@@ -25,6 +25,7 @@
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/Instruction.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/NoFolder.h"
 #include "llvm/Support/CommandLine.h"
@@ -5526,8 +5527,9 @@ struct AAPrivatizablePtrArgument final : public AAPrivatizablePtrImpl {
         new StoreInst(F.getArg(ArgNo + u), Ptr, &IP);
       }
     } else if (auto *PrivArrayType = dyn_cast<ArrayType>(PrivType)) {
-      Type *PointeePtrTy = PrivArrayType->getElementType()->getPointerTo();
-      uint64_t PointeeTySize = DL.getTypeStoreSize(PointeePtrTy);
+      Type *PointeeTy = PrivArrayType->getElementType();
+      Type *PointeePtrTy = PointeeTy->getPointerTo();
+      uint64_t PointeeTySize = DL.getTypeStoreSize(PointeeTy);
       for (unsigned u = 0, e = PrivArrayType->getNumElements(); u < e; u++) {
         Value *Ptr =
             constructPointer(PointeePtrTy, &Base, u * PointeeTySize, IRB, DL);
@@ -5573,7 +5575,7 @@ struct AAPrivatizablePtrArgument final : public AAPrivatizablePtrImpl {
       for (unsigned u = 0, e = PrivArrayType->getNumElements(); u < e; u++) {
         Value *Ptr =
             constructPointer(PointeePtrTy, Base, u * PointeeTySize, IRB, DL);
-        LoadInst *L = new LoadInst(PointeePtrTy, Ptr, "", IP);
+        LoadInst *L = new LoadInst(PointeeTy, Ptr, "", IP);
         L->setAlignment(Alignment);
         ReplacementValues.push_back(L);
       }
@@ -5617,10 +5619,14 @@ struct AAPrivatizablePtrArgument final : public AAPrivatizablePtrImpl {
             Function &ReplacementFn, Function::arg_iterator ArgIt) {
           BasicBlock &EntryBB = ReplacementFn.getEntryBlock();
           Instruction *IP = &*EntryBB.getFirstInsertionPt();
-          auto *AI = new AllocaInst(PrivatizableType.getValue(), 0,
-                                    Arg->getName() + ".priv", IP);
+          Instruction *AI = new AllocaInst(PrivatizableType.getValue(), 0,
+                                           Arg->getName() + ".priv", IP);
           createInitialization(PrivatizableType.getValue(), *AI, ReplacementFn,
                                ArgIt->getArgNo(), *IP);
+
+          if (AI->getType() != Arg->getType())
+            AI =
+                BitCastInst::CreateBitOrPointerCast(AI, Arg->getType(), "", IP);
           Arg->replaceAllUsesWith(AI);
 
           for (CallInst *CI : TailCalls)
diff --git a/llvm/test/Transforms/Attributor/ArgumentPromotion/array.ll b/llvm/test/Transforms/Attributor/ArgumentPromotion/array.ll
new file mode 100644 (file)
index 0000000..b3cc401
--- /dev/null
@@ -0,0 +1,76 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --check-attributes
+; RUN: opt -attributor -enable-new-pm=0 -attributor-manifest-internal  -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=3 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_NPM,NOT_CGSCC_OPM,NOT_TUNIT_NPM,IS__TUNIT____,IS________OPM,IS__TUNIT_OPM
+; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal  -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=3 -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_CGSCC_OPM,NOT_CGSCC_NPM,NOT_TUNIT_OPM,IS__TUNIT____,IS________NPM,IS__TUNIT_NPM
+; RUN: opt -attributor-cgscc -enable-new-pm=0 -attributor-manifest-internal  -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_TUNIT_NPM,NOT_TUNIT_OPM,NOT_CGSCC_NPM,IS__CGSCC____,IS________OPM,IS__CGSCC_OPM
+; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -attributor-manifest-internal  -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,NOT_TUNIT_NPM,NOT_TUNIT_OPM,NOT_CGSCC_OPM,IS__CGSCC____,IS________NPM,IS__CGSCC_NPM
+;
+; FIXME: The GEP + BC + GEP solution we create is not great but correct.
+
+declare void @use(i32* nocapture readonly %arg)
+
+define void @caller() {
+; IS________OPM-LABEL: define {{[^@]+}}@caller() {
+; IS________OPM-NEXT:  entry:
+; IS________OPM-NEXT:    [[LEFT:%.*]] = alloca [3 x i32], align 4
+; IS________OPM-NEXT:    [[ARRAYDECAY:%.*]] = getelementptr inbounds [3 x i32], [3 x i32]* [[LEFT]], i64 0, i64 0
+; IS________OPM-NEXT:    call void @callee(i32* noalias nocapture noundef nonnull readonly align 4 dereferenceable(12) [[ARRAYDECAY]])
+; IS________OPM-NEXT:    ret void
+;
+; IS________NPM-LABEL: define {{[^@]+}}@caller() {
+; IS________NPM-NEXT:  entry:
+; IS________NPM-NEXT:    [[LEFT:%.*]] = alloca [3 x i32], align 4
+; IS________NPM-NEXT:    [[ARRAYDECAY:%.*]] = getelementptr inbounds [3 x i32], [3 x i32]* [[LEFT]], i64 0, i64 0
+; IS________NPM-NEXT:    [[TMP0:%.*]] = bitcast i32* [[ARRAYDECAY]] to [3 x i32]*
+; IS________NPM-NEXT:    [[DOTCAST:%.*]] = bitcast [3 x i32]* [[TMP0]] to i32*
+; IS________NPM-NEXT:    [[TMP1:%.*]] = load i32, i32* [[DOTCAST]], align 4
+; IS________NPM-NEXT:    [[DOT0:%.*]] = getelementptr [3 x i32], [3 x i32]* [[TMP0]], i32 0
+; IS________NPM-NEXT:    [[TMP2:%.*]] = bitcast [3 x i32]* [[DOT0]] to i8*
+; IS________NPM-NEXT:    [[DOT0_B4:%.*]] = getelementptr i8, i8* [[TMP2]], i32 4
+; IS________NPM-NEXT:    [[DOT0_B4_CAST:%.*]] = bitcast i8* [[DOT0_B4]] to i32*
+; IS________NPM-NEXT:    [[TMP3:%.*]] = load i32, i32* [[DOT0_B4_CAST]], align 4
+; IS________NPM-NEXT:    [[DOT01:%.*]] = getelementptr [3 x i32], [3 x i32]* [[TMP0]], i32 0
+; IS________NPM-NEXT:    [[TMP4:%.*]] = bitcast [3 x i32]* [[DOT01]] to i8*
+; IS________NPM-NEXT:    [[DOT0_B8:%.*]] = getelementptr i8, i8* [[TMP4]], i32 8
+; IS________NPM-NEXT:    [[DOT0_B8_CAST:%.*]] = bitcast i8* [[DOT0_B8]] to i32*
+; IS________NPM-NEXT:    [[TMP5:%.*]] = load i32, i32* [[DOT0_B8_CAST]], align 4
+; IS________NPM-NEXT:    call void @callee(i32 [[TMP1]], i32 [[TMP3]], i32 [[TMP5]])
+; IS________NPM-NEXT:    ret void
+;
+entry:
+  %left = alloca [3 x i32], align 4
+  %arraydecay = getelementptr inbounds [3 x i32], [3 x i32]* %left, i64 0, i64 0
+  call void @callee(i32* %arraydecay)
+  ret void
+}
+
+define internal void @callee(i32* noalias %arg) {
+; IS________OPM-LABEL: define {{[^@]+}}@callee
+; IS________OPM-SAME: (i32* noalias nocapture noundef nonnull readonly align 4 dereferenceable(12) [[ARG:%.*]]) {
+; IS________OPM-NEXT:  entry:
+; IS________OPM-NEXT:    call void @use(i32* noalias nocapture noundef nonnull readonly align 4 dereferenceable(12) [[ARG]])
+; IS________OPM-NEXT:    ret void
+;
+; IS________NPM-LABEL: define {{[^@]+}}@callee
+; IS________NPM-SAME: (i32 [[TMP0:%.*]], i32 [[TMP1:%.*]], i32 [[TMP2:%.*]]) {
+; IS________NPM-NEXT:  entry:
+; IS________NPM-NEXT:    [[ARG_PRIV:%.*]] = alloca [3 x i32], align 4
+; IS________NPM-NEXT:    [[ARG_PRIV_CAST:%.*]] = bitcast [3 x i32]* [[ARG_PRIV]] to i32*
+; IS________NPM-NEXT:    store i32 [[TMP0]], i32* [[ARG_PRIV_CAST]], align 4
+; IS________NPM-NEXT:    [[ARG_PRIV_0:%.*]] = getelementptr [3 x i32], [3 x i32]* [[ARG_PRIV]], i32 0
+; IS________NPM-NEXT:    [[TMP3:%.*]] = bitcast [3 x i32]* [[ARG_PRIV_0]] to i8*
+; IS________NPM-NEXT:    [[ARG_PRIV_0_B4:%.*]] = getelementptr i8, i8* [[TMP3]], i32 4
+; IS________NPM-NEXT:    [[ARG_PRIV_0_B4_CAST:%.*]] = bitcast i8* [[ARG_PRIV_0_B4]] to i32*
+; IS________NPM-NEXT:    store i32 [[TMP1]], i32* [[ARG_PRIV_0_B4_CAST]], align 4
+; IS________NPM-NEXT:    [[ARG_PRIV_01:%.*]] = getelementptr [3 x i32], [3 x i32]* [[ARG_PRIV]], i32 0
+; IS________NPM-NEXT:    [[TMP4:%.*]] = bitcast [3 x i32]* [[ARG_PRIV_01]] to i8*
+; IS________NPM-NEXT:    [[ARG_PRIV_0_B8:%.*]] = getelementptr i8, i8* [[TMP4]], i32 8
+; IS________NPM-NEXT:    [[ARG_PRIV_0_B8_CAST:%.*]] = bitcast i8* [[ARG_PRIV_0_B8]] to i32*
+; IS________NPM-NEXT:    store i32 [[TMP2]], i32* [[ARG_PRIV_0_B8_CAST]], align 4
+; IS________NPM-NEXT:    [[TMP5:%.*]] = bitcast [3 x i32]* [[ARG_PRIV]] to i32*
+; IS________NPM-NEXT:    call void @use(i32* noalias nocapture noundef nonnull readonly align 4 dereferenceable(12) [[TMP5]])
+; IS________NPM-NEXT:    ret void
+;
+entry:
+  call void @use(i32* %arg)
+  ret void
+}