[PATCH] D115829: Allow calls with known writes when trying to remove allocas

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:2574
+
+  if (CB.hasOperandBundles())
+    return false;
----------------
Isn't this already covered by `onlyAccessesArgMemory()`? If the operand bundle touches other memory, then that would return false.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:2580
+      continue;
+    if (UsedV != CB.getArgOperand(i) && !CB.onlyReadsMemory(i))
+      // A write to another memory location keeps the call live, and thus we
----------------
The logic here doesn't look right because you're conflating two different cases. If you have a non-UsedV argument that is readonly, then you want to skip that, but currently it will fail because you then go to the code below, which also checks for writeonly. You should be checking nocapture (always) and then for UsedV writeonly and non-UsedV readonly. A test case for a readonly argument seems to be missing as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115829