[PATCH] D150900: [InstCombine] Insert a bitcast to enable merging similar store insts

[Nikita Popov via Phabricator via llvm-commits]

nikic requested changes to this revision.
nikic added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:1594
+        !OSVTy->isSingleValueType() || OSVTy->isVectorTy() ||
+        SIVTy->getPrimitiveSizeInBits() != OSVTy->getPrimitiveSizeInBits())
+      return nullptr;
----------------
Check for `isBitOrNoopPointerCastable()` instead, and please add a test that requires inserting ptrtoint or inttoptr.

Also this is missing a check for hasSameSpecialState(). Please add a test with for example different alignments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:1602
+    auto *NewOtherStore = new StoreInst(NewVal, OtherStore->getPointerOperand(),
+                                        OtherStore->getParent());
+
----------------
You should only insert the bitcast when actually doing the transform. We should not inserting if the transform is aborted later.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150900/new/

https://reviews.llvm.org/D150900