[PATCH] D15210: [WebAssembly] Fix dominance check for PHIs in the StoreResult pass

Dan Gohman via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 3 15:12:24 PST 2015


sunfish added a comment.

Thanks!


================
Comment at: lib/Target/WebAssembly/WebAssemblyStoreResults.cpp:95
@@ +94,3 @@
+            // is used.
+            MachineBasicBlock *Pred = Where->getOperand(&O - &Where->getOperand(0) + 1).getMBB();
+            if (!MDT.dominates(&MBB, Pred))
----------------
dschuff wrote:
> I don't really understand what this arithmetic on the MachineOperands actually means.
> I guess it's just trying to find which operand on Where is the use of MI.
> 
> Actually there's also nothing that explicitly says what the pass even does.
> IIUC we are attempting to find uses of the stored value and make them uses of the result of the store instead (which we can do if the store dominates the use), so maybe it should say that somewhere.
Yes, it's computing the block paired with a use in the phi. I expanded the comment.

The comment at the top of the file describes what this pass does, but it was pretty terse. I expanded it now.


================
Comment at: test/CodeGen/WebAssembly/store-results.ll:26
@@ +25,3 @@
+
+; CHECK-LABEL: foo:
+; CHECK: i32.store $discard=, $pop0, $0
----------------
dschuff wrote:
> could this maybe say what the nature of the corner case is for each of these functions?
I expanded the comment describing these tests.


Repository:
  rL LLVM

http://reviews.llvm.org/D15210





More information about the llvm-commits mailing list