[ConstantFolding] Use FoldBitCast correctly

[vtjnash]

Summary

Previously we might attempt to use a BitCast to turn bits into vectors of pointers,
but that requires an inttoptr cast to be legal. Add an assertion to detect the formation of illegal bitcast attempts
early (in the tests, we often constant-fold away the result before getting to this assertion check),
while being careful to still handle the early-return conditions without adding extra complexity in the result.

[lebedev.ri]

Test?

[vtjnash]

The added assertion will detect this:

[109/110] Running lit suite /data/vtjnash/julia/deps/srccache/llvm-svn/test/Transforms                                                                                       [19/519]
FAIL: LLVM :: Transforms/InstSimplify/vector_ptr_bitcast.ll (2527 of 4814)
******************** TEST 'LLVM :: Transforms/InstSimplify/vector_ptr_bitcast.ll' FAILED ********************
Script:
--
: 'RUN: at line 1';   /data/vtjnash/julia/deps/scratch/llvm-svn/build_Release+Asserts/bin/opt -S -instsimplify < /data/vtjnash/julia/deps/srccache/llvm-svn/test/Transforms/InstSimpl
ify/vector_ptr_bitcast.ll | /data/vtjnash/julia/deps/scratch/llvm-svn/build_Release+Asserts/bin/FileCheck /data/vtjnash/julia/deps/srccache/llvm-svn/test/Transforms/InstSimplify/vec
tor_ptr_bitcast.ll
--
Exit Code: 2

Command Output (stderr):
--
opt: /data/vtjnash/julia/deps/srccache/llvm-svn/lib/Analysis/ConstantFolding.cpp:97: llvm::Constant* {anonymous}::FoldBitCast(llvm::Constant*, llvm::Type*, const llvm::DataLayout&):
 Assertion `CastInst::castIsValid(Instruction::BitCast, C, DestTy) && "Invalid constantexpr bitcast!"' failed.
Stack dump:
0.      Program arguments: /data/vtjnash/julia/deps/scratch/llvm-svn/build_Release+Asserts/bin/opt -S -instsimplify
1.      Running pass 'Function Pass Manager' on module '<stdin>'.
2.      Running pass 'Remove redundant instructions' on function '@fn'
 #0 0x00007f19a6bdb9aa llvm::sys::PrintStackTrace(llvm::raw_ostream&) (/data/vtjnash/julia/deps/scratch/llvm-svn/build_Release+Asserts/bin/../lib/libLLVM-10svn.so+0x8cd9aa)
 #1 0x00007f19a6bd9654 llvm::sys::RunSignalHandlers() (/data/vtjnash/julia/deps/scratch/llvm-svn/build_Release+Asserts/bin/../lib/libLLVM-10svn.so+0x8cb654)
 #2 0x00007f19a6bd9792 SignalHandler(int) (/data/vtjnash/julia/deps/scratch/llvm-svn/build_Release+Asserts/bin/../lib/libLLVM-10svn.so+0x8cb792)
 #3 0x00007f19a59baf20 (/lib/x86_64-linux-gnu/libc.so.6+0x3ef20)
 #4 0x00007f19a59bae97 raise /build/glibc-OTsEL5/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0
 #5 0x00007f19a59bc801 abort /build/glibc-OTsEL5/glibc-2.27/stdlib/abort.c:81:0
 #6 0x00007f19a59ac39a __assert_fail_base /build/glibc-OTsEL5/glibc-2.27/assert/assert.c:89:0
 #7 0x00007f19a59ac412 (/lib/x86_64-linux-gnu/libc.so.6+0x30412)
 #8 0x00007f19a7d062b3 (/data/vtjnash/julia/deps/scratch/llvm-svn/build_Release+Asserts/bin/../lib/libLLVM-10svn.so+0x19f82b3)
 #9 0x00007f19a7d06f21 (anonymous namespace)::FoldReinterpretLoadFromConstPtr(llvm::Constant*, llvm::Type*, llvm::DataLayout const&) (/data/vtjnash/julia/deps/scratch/llvm-svn/build
_Release+Asserts/bin/../lib/libLLVM-10svn.so+0x19f8f21)
#10 0x00007f19a7d0730f llvm::ConstantFoldLoadFromConstPtr(llvm::Constant*, llvm::Type*, llvm::DataLayout const&) (/data/vtjnash/julia/deps/scratch/llvm-svn/build_Release+Asserts/bin
/../lib/libLLVM-10svn.so+0x19f930f)
#11 0x00007f19a7d0dd01 llvm::ConstantFoldInstruction(llvm::Instruction*, llvm::DataLayout const&, llvm::TargetLibraryInfo const*) (/data/vtjnash/julia/deps/scratch/llvm-svn/build_Re
lease+Asserts/bin/../lib/libLLVM-10svn.so+0x19ffd01)
#12 0x00007f19a7d80f0a llvm::SimplifyInstruction(llvm::Instruction*, llvm::SimplifyQuery const&, llvm::OptimizationRemarkEmitter*) (/data/vtjnash/julia/deps/scratch/llvm-svn/build_R
elease+Asserts/bin/../lib/libLLVM-10svn.so+0x1a72f0a)
#13 0x00007f19a791841f runImpl(llvm::Function&, llvm::SimplifyQuery const&, llvm::OptimizationRemarkEmitter*) (.constprop.115) (/data/vtjnash/julia/deps/scratch/llvm-svn/build_Relea
se+Asserts/bin/../lib/libLLVM-10svn.so+0x160a41f)
#14 0x00007f19a791a924 (anonymous namespace)::InstSimplifyLegacyPass::runOnFunction(llvm::Function&) (/data/vtjnash/julia/deps/scratch/llvm-svn/build_Release+Asserts/bin/../lib/libL
LVM-10svn.so+0x160c924)
#15 0x00007f19a6d0e359 llvm::FPPassManager::runOnFunction(llvm::Function&) (/data/vtjnash/julia/deps/scratch/llvm-svn/build_Release+Asserts/bin/../lib/libLLVM-10svn.so+0xa00359)
#16 0x00007f19a6d0e451 llvm::FPPassManager::runOnModule(llvm::Module&) (/data/vtjnash/julia/deps/scratch/llvm-svn/build_Release+Asserts/bin/../lib/libLLVM-10svn.so+0xa00451)
#17 0x00007f19a6d0d5b1 llvm::legacy::PassManagerImpl::run(llvm::Module&) (/data/vtjnash/julia/deps/scratch/llvm-svn/build_Release+Asserts/bin/../lib/libLLVM-10svn.so+0x9ff5b1)
#18 0x000055efb10b7765 main (/data/vtjnash/julia/deps/scratch/llvm-svn/build_Release+Asserts/bin/opt+0x1e765)
#19 0x00007f19a599db97 __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:344:0
#20 0x000055efb10b802a _start (/data/vtjnash/julia/deps/scratch/llvm-svn/build_Release+Asserts/bin/opt+0x1f02a)
FileCheck error: '-' is empty.
FileCheck command line:  /data/vtjnash/julia/deps/scratch/llvm-svn/build_Release+Asserts/bin/FileCheck /data/vtjnash/julia/deps/srccache/llvm-svn/test/Transforms/InstSimplify/vector
_ptr_bitcast.ll

[vtjnash]

Friendly bump.

[vtjnash]

bump.

[lebedev.ri]

lib/Analysis/ConstantFolding.cpp

535–536 ↗

(On Diff #211000)

But didn't you just check that before?

[vtjnash]

lib/Analysis/ConstantFolding.cpp

535–536 ↗

(On Diff #211000)

We did, but on a different value.

[vtjnash]

bump again? (I don't have commit rights.)

[vtjnash]

Bump again? It's just a tiny bugfix that reames told me is needed (https://reviews.llvm.org/D59730?id=196652#inline-570528)

[vtjnash]

bump

[vtjnash]

bump? please review/merge this bugfix?

[loladiro]

LGTM. @lebedev.ri, do you have further review?

[lebedev.ri]

In D65057#1671249, @loladiro wrote:

LGTM. @lebedev.ri, do you have further review?

No, i don't think i have any further comments here.

llvm/trunk/lib/Analysis/ConstantFolding.cpp

static Constant *foldConstVectorToAPInt(APInt &Result, Type *DestTy,
 
   return nullptr;
 }
 
 /// Constant fold bitcast, symbolically evaluating it with DataLayout.
 /// This always returns a non-null constant, but it may be a
 /// ConstantExpr if unfoldable.
 Constant *FoldBitCast(Constant *C, Type *DestTy, const DataLayout &DL) {
+  assert(CastInst::castIsValid(Instruction::BitCast, C, DestTy) &&
+         "Invalid constantexpr bitcast!");
+
   // Catch the obvious splat cases.
   if (C->isNullValue() && !DestTy->isX86_MMXTy())
     return Constant::getNullValue(DestTy);
   if (C->isAllOnesValue() && !DestTy->isX86_MMXTy() &&
       !DestTy->isPtrOrPtrVectorTy()) // Don't get ones for ptr types!
     return Constant::getAllOnesValue(DestTy);
 
   if (auto *VTy = dyn_cast<VectorType>(C->getType())) {
else if (LoadTy->isDoubleTy())
       MapTy = Type::getInt64Ty(C->getContext());
     else if (LoadTy->isVectorTy()) {
       MapTy = PointerType::getIntNTy(C->getContext(),
                                      DL.getTypeSizeInBits(LoadTy));
     } else
       return nullptr;
 
     C = FoldBitCast(C, MapTy->getPointerTo(AS), DL);
-    if (Constant *Res = FoldReinterpretLoadFromConstPtr(C, MapTy, DL))
-      return FoldBitCast(Res, LoadTy, DL);
+    if (Constant *Res = FoldReinterpretLoadFromConstPtr(C, MapTy, DL)) {
+      if (Res->isNullValue() && !LoadTy->isX86_MMXTy())
+        // Materializing a zero can be done trivially without a bitcast
+        return Constant::getNullValue(LoadTy);
+      Type *CastTy = LoadTy->isPtrOrPtrVectorTy() ? DL.getIntPtrType(LoadTy) : LoadTy;
+      Res = FoldBitCast(Res, CastTy, DL);
+      if (LoadTy->isPtrOrPtrVectorTy()) {
+        // For vector of pointer, we needed to first convert to a vector of integer, then do vector inttoptr
+        if (Res->isNullValue() && !LoadTy->isX86_MMXTy())
+          return Constant::getNullValue(LoadTy);
+        if (DL.isNonIntegralPointerType(LoadTy->getScalarType()))
+          // Be careful not to replace a load of an addrspace value with an inttoptr here
+          return nullptr;
+        Res = ConstantExpr::getCast(Instruction::IntToPtr, Res, LoadTy);
+      }
+      return Res;
+    }
     return nullptr;
   }
 
   unsigned BytesLoaded = (IntType->getBitWidth() + 7) / 8;
   if (BytesLoaded > 32 || BytesLoaded == 0)
     return nullptr;
 
   GlobalValue *GVal;