diff options
author | Eli Friedman <efriedma@quicinc.com> | 2025-02-25 15:29:12 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2025-02-25 15:29:12 -0800 |
commit | 1b39328d7440aa7a94af4083257ef1c2f9394887 (patch) | |
tree | 12c96248d8b7b1714952ae40bbfe3fc217e9b365 /llvm/lib/CodeGen/MachineInstr.cpp | |
parent | fc655b1ae78305ad0839c0311f72607775af0c73 (diff) | |
download | llvm-1b39328d7440aa7a94af4083257ef1c2f9394887.zip llvm-1b39328d7440aa7a94af4083257ef1c2f9394887.tar.gz llvm-1b39328d7440aa7a94af4083257ef1c2f9394887.tar.bz2 |
[CodeGen] Fix MachineInstr::isSafeToMove handling of inline asm. (#126807)
Even if an inline asm doesn't have memory effects, we can't assume it's
safe to speculate: it could trap, or cause undefined behavior. At the
LLVM IR level, this is handled correctly: we don't speculate inline asm
(unless it's marked "speculatable", but I don't think anyone does that).
Codegen also needs to respect this restriction.
This change stops Early If Conversion and similar passes from
speculating an INLINEASM MachineInstr.
Some uses of isSafeToMove probably could be switched to a different API:
isSafeToMove assumes you're hoisting, but we could handle some forms of
sinking more aggressively. But I'll leave that for a followup, if it
turns out to be relevant.
See also discussion on gcc bugtracker
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102150 .
Diffstat (limited to 'llvm/lib/CodeGen/MachineInstr.cpp')
-rw-r--r-- | llvm/lib/CodeGen/MachineInstr.cpp | 19 |
1 files changed, 18 insertions, 1 deletions
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp index 52c977a..5860a76 100644 --- a/llvm/lib/CodeGen/MachineInstr.cpp +++ b/llvm/lib/CodeGen/MachineInstr.cpp @@ -1308,11 +1308,28 @@ bool MachineInstr::isSafeToMove(bool &SawStore) const { return false; } + // Don't touch instructions that have non-trivial invariants. For example, + // terminators have to be at the end of a basic block. if (isPosition() || isDebugInstr() || isTerminator() || - mayRaiseFPException() || hasUnmodeledSideEffects() || isJumpTableDebugInfo()) return false; + // Don't touch instructions which can have non-load/store effects. + // + // Inline asm has a "sideeffect" marker to indicate whether the asm has + // intentional side-effects. Even if an inline asm is not "sideeffect", + // though, it still can't be speculatively executed: the operation might + // not be valid on the current target, or for some combinations of operands. + // (Some transforms that move an instruction don't speculatively execute it; + // we currently don't try to handle that distinction here.) + // + // Other instructions handled here include those that can raise FP + // exceptions, x86 "DIV" instructions which trap on divide by zero, and + // stack adjustments. + if (mayRaiseFPException() || hasProperty(MCID::UnmodeledSideEffects) || + isInlineAsm()) + return false; + // See if this instruction does a load. If so, we have to guarantee that the // loaded value doesn't change between the load and the its intended // destination. The check for isInvariantLoad gives the target the chance to |