[PATCH] D51751: Merge clang's isRepeatedBytePattern with LLVM's isBytewiseValue

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Analysis/ValueTracking.cpp:3121
+
+  if (isa<ConstantArray>(C) || isa<ConstantStruct>(C)) {
+    Value *Val = UndefInt8;
----------------
jfb wrote:
> efriedma wrote:
> > Do you really need separate loops for ConstantVector and ConstantArray?
> I don't think ConstantArray has getSplatValue, which I use for ConstantVector. Or are you suggesting something else?
I meant you could just use the for loop for ConstantVector.  Maybe a bit less efficient, but the difference is unlikely to matter.


================
Comment at: lib/Transforms/Scalar/MemCpyOptimizer.cpp:391
                                                  Value *StartPtr,
-                                                 Value *ByteVal) {
+                                                 Value *&ByteVal) {
   const DataLayout &DL = StartInst->getModule()->getDataLayout();
----------------
jfb wrote:
> efriedma wrote:
> > What's the point of changing this signature?  It doesn't seem to have any effect.
> If ByteVal is UndefValue then it can be merged with any other valid pattern. The below change does this. The signature change allows updating ByteVal. I could instead return a pair if you want.
Whether you use an out parameter or a pair, the value still isn't actually used by either of the callers of tryMergingIntoMemset.

Looking again, I guess technically it's used in MemCpyOptPass::processStore, but only in the case where tryMergingIntoMemset fails, so I'm guessing that isn't intentional.


Repository:
  rL LLVM

https://reviews.llvm.org/D51751