[PATCH] D30416: [InstCombine] Redo reduceLoadOpStoreWidth in instcombine for bitfield store optimization.

[Wei Mi via Phabricator via llvm-commits]

wmi added a comment.

> Could you add a test-case like this?

Sure. I will add such testcase after other major issues are solved.



================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:5739
+  // Limit the maximum number of BasicBlocks to 3 to protect compile time.
+  const int MaxBB = 3;
+  SmallPtrSet<const BasicBlock *, 4> BBSet;
----------------
mkuper wrote:
> Oh, ok, now I see why Eli suggested MemorySSA.
> There has to be a better way to do this. (Although I can't think of one at the moment.)
Yes, if the optimization happens before loadpre, then this simple check is enough. If the optimization happens late in the pipeline, we need memoryssa + alias query to do the safety check. 



================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:5895
+  unsigned TBits = DL.getTypeStoreSizeInBits(StoreTy);
+  if (TBits != DL.getTypeStoreSizeInBits(StoreTy))
+    return false;
----------------
mkuper wrote:
> I don't think this is what you meant to check here. :-)
Oh, thanks for catching the stupid mistake.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:5912
+  // LI should have the same address as SI.
+  if (LI->getOperand(0) != Ptr)
+    return false;
----------------
mkuper wrote:
> Do we want to look through bitcasts? It probably doesn't matter in practice, though.
Yes, I want. I do see case that needs it.


Repository:
  rL LLVM

https://reviews.llvm.org/D30416