[PATCH] Don't report the error when non-dangerous load/store is in branch delay slot

Mark Seaborn mseaborn at chromium.org
Fri Jun 6 16:46:47 PDT 2014


================
Comment at: lib/Target/Mips/MCTargetDesc/MipsNaClELFStreamer.cpp:151
@@ +150,3 @@
+
+    // Sandbox loads, stores and SP changes.
+    unsigned AddrIdx;
----------------
Sasa Stankovic wrote:
> Mark Seaborn wrote:
> > I don't get why you've reordered this chunk of code.  Surely the ordering doesn't matter, because Inst can't be both a call and a memory access on MIPS, can it?
> If for the load/store or stack change instruction the condition (MaskBefore || MaskAfter) is false, then in the original ordering the code would first unnecessary check whether the instruction is call (it isn't), and then would check whether the instruction is in the branch delay slot ("if (PendingCall)"). With the ordering in the patch, the unnecessary check whether the instruction is call is avoided.
I don't get it.  Surely if you have an instruction for which "IsMemAccess || IsSPFirstOperand" is true but "MaskBefore || MaskAfter" is false, then the "isCall(Inst.getOpcode(), &IsIndirectCall)" check is going to get executed regardless of the ordering of these two chunks of code?  Is that what you mean?

Can you restore the original ordering to make the correctness fix clearer, please?

If I've misunderstood this, and the ordering affects compile time (or code clarity), could you do the reordering as a separate change?

http://reviews.llvm.org/D4048






More information about the llvm-commits mailing list