[PATCH] D25914: Redo store splitting in CodeGenPrepare

[Chandler Carruth via llvm-commits]

chandlerc added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:5311-5312
+  // one use.
+  Value *LValue = nullptr;
+  Value *HValue = nullptr;
+  if (!match(SI.getValueOperand(),
----------------
No need to initialize these. Leaving them uninitialized makes it easier for MSan to find bugs if we try to use them despite the match failing. (Or a bug in the match that fails to set them.)


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:5342-5350
+  // If LValue/HValue is a bitcast in another BB and has only one use, move
+  // it to current BB so it may be merged with the splitted stores by dag
+  // combiner.
+  BitCastInst *LBC = dyn_cast<BitCastInst>(LValue);
+  BitCastInst *HBC = dyn_cast<BitCastInst>(HValue);
+  if (LBC && LBC->hasOneUse() && LBC->getParent() != SI.getParent())
+    LValue = Builder.CreateBitCast(LBC->getOperand(0), LBC->getType());
----------------
What about just walking LValue and HValue back across any bitcast instructions? That should make it easier to get the type for the cost query, and then make this simpler as you can directly store the pre-bitcast value.

I also wouldn't worry about how many uses of the bitcast there are either, as bitcasts are expected to be free. As one example, if there are 3 uses of the bitcast, all to do merged stores, we should be willing to unmerge all three of them.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:5352
+
+  Type *Ty = Type::getIntNTy(SI.getContext(), HalfValBitSize);
+  Type *PtrTy = Ty->getPointerTo(SI.getPointerAddressSpace());
----------------
Just use the type of the input value rather than re-computing?

Also, you can probably just use a lambda to create the store and then call it for each of the inputs?


Repository:
  rL LLVM

https://reviews.llvm.org/D25914