Skip to content

Commit

Permalink
[TargetInstrInfo] enable foldMemoryOperand for InlineAsm (#70743)
Browse files Browse the repository at this point in the history
foldMemoryOperand looks at pairs of instructions (generally a load to
virt reg then use of the virtreg, or def of a virtreg then a store) and
attempts to combine them.  This can reduce register pressure.

A prior commit added the ability to mark such a MachineOperand as
foldable. In terms of INLINEASM, this means that "rm" was used (rather
than just "r") to denote that the INLINEASM may use a memory operand
rather than a register operand. This effectively undoes decisions made
by the instruction selection framework.  Callers will be added in the
register allocation frameworks. This has been tested with all of the
above (which will come as follow up patches).

Thanks to @topperc who suggested this at last years LLVM US Dev Meeting
and @qcolombet who confirmed this was the right approach.

Link: #20571
  • Loading branch information
nickdesaulniers committed Nov 17, 2023
1 parent f99a020 commit 99ee2db
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 0 deletions.
10 changes: 10 additions & 0 deletions llvm/include/llvm/CodeGen/TargetInstrInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -2188,6 +2188,16 @@ class TargetInstrInfo : public MCInstrInfo {
// Get the call frame size just before MI.
unsigned getCallFrameSizeAt(MachineInstr &MI) const;

/// Fills in the necessary MachineOperands to refer to a frame index.
/// The best way to understand this is to print `asm(""::"m"(x));` after
/// finalize-isel. Example:
/// INLINEASM ... 262190 /* mem:m */, %stack.0.x.addr, 1, $noreg, 0, $noreg
/// we would add placeholders for: ^ ^ ^ ^
virtual void
getFrameIndexOperands(SmallVectorImpl<MachineOperand> &Ops) const {
llvm_unreachable("unknown number of operands necessary");
}

private:
mutable std::unique_ptr<MIRFormatter> Formatter;
unsigned CallFrameSetupOpcode, CallFrameDestroyOpcode;
Expand Down
62 changes: 62 additions & 0 deletions llvm/lib/CodeGen/TargetInstrInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,64 @@ static MachineInstr *foldPatchpoint(MachineFunction &MF, MachineInstr &MI,
return NewMI;
}

static void foldInlineAsmMemOperand(MachineInstr *MI, unsigned OpNo, int FI,
const TargetInstrInfo &TII) {
MachineOperand &MO = MI->getOperand(OpNo);
const VirtRegInfo &RI = AnalyzeVirtRegInBundle(*MI, MO.getReg());

// If the machine operand is tied, untie it first.
if (MO.isTied()) {
unsigned TiedTo = MI->findTiedOperandIdx(OpNo);
MI->untieRegOperand(OpNo);
// Intentional recursion!
foldInlineAsmMemOperand(MI, TiedTo, FI, TII);
}

// Change the operand from a register to a frame index.
MO.ChangeToFrameIndex(FI, MO.getTargetFlags());

SmallVector<MachineOperand, 4> NewOps;
TII.getFrameIndexOperands(NewOps);
assert(!NewOps.empty() && "getFrameIndexOperands didn't create any operands");
MI->insert(MI->operands_begin() + OpNo + 1, NewOps);

// Change the previous operand to a MemKind InlineAsm::Flag. The second param
// is the per-target number of operands that represent the memory operand
// excluding this one (MD). This includes MO.
InlineAsm::Flag F(InlineAsm::Kind::Mem, NewOps.size() + 1);
F.setMemConstraint(InlineAsm::ConstraintCode::m);
MachineOperand &MD = MI->getOperand(OpNo - 1);
MD.setImm(F);

// Update mayload/maystore metadata.
MachineOperand &ExtraMO = MI->getOperand(InlineAsm::MIOp_ExtraInfo);
if (RI.Reads)
ExtraMO.setImm(ExtraMO.getImm() | InlineAsm::Extra_MayLoad);
if (RI.Writes)
ExtraMO.setImm(ExtraMO.getImm() | InlineAsm::Extra_MayStore);
}

// Returns nullptr if not possible to fold.
static MachineInstr *foldInlineAsmMemOperand(MachineInstr &MI,
ArrayRef<unsigned> Ops, int FI,
const TargetInstrInfo &TII) {
assert(MI.isInlineAsm() && "wrong opcode");
if (Ops.size() > 1)
return nullptr;
unsigned Op = Ops[0];
assert(Op && "should never be first operand");
assert(MI.getOperand(Op).isReg() && "shouldn't be folding non-reg operands");

if (!MI.mayFoldInlineAsmRegOp(Op))
return nullptr;

MachineInstr &NewMI = TII.duplicate(*MI.getParent(), MI.getIterator(), MI);

foldInlineAsmMemOperand(&NewMI, Op, FI, TII);

return &NewMI;
}

MachineInstr *TargetInstrInfo::foldMemoryOperand(MachineInstr &MI,
ArrayRef<unsigned> Ops, int FI,
LiveIntervals *LIS,
Expand Down Expand Up @@ -612,6 +670,8 @@ MachineInstr *TargetInstrInfo::foldMemoryOperand(MachineInstr &MI,
NewMI = foldPatchpoint(MF, MI, Ops, FI, *this);
if (NewMI)
MBB->insert(MI, NewMI);
} else if (MI.isInlineAsm()) {
NewMI = foldInlineAsmMemOperand(MI, Ops, FI, *this);
} else {
// Ask the target to do the actual folding.
NewMI = foldMemoryOperandImpl(MF, MI, Ops, MI, FI, LIS, VRM);
Expand Down Expand Up @@ -683,6 +743,8 @@ MachineInstr *TargetInstrInfo::foldMemoryOperand(MachineInstr &MI,
NewMI = foldPatchpoint(MF, MI, Ops, FrameIndex, *this);
if (NewMI)
NewMI = &*MBB.insert(MI, NewMI);
} else if (MI.isInlineAsm() && isLoadFromStackSlot(LoadMI, FrameIndex)) {
NewMI = foldInlineAsmMemOperand(MI, Ops, FrameIndex, *this);
} else {
// Ask the target to do the actual folding.
NewMI = foldMemoryOperandImpl(MF, MI, Ops, MI, LoadMI, LIS);
Expand Down

4 comments on commit 99ee2db

@bwendling
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bisect says that this is failing many ARM tests (see below). I'm going to do a revert. :-(

$ ninja bin/clang && /home/morbo/llvm/llvm-project/build.opt/bin/clang -cc1 -internal-isystem /home/morbo/llvm/llvm-project/build.opt/lib/clang/18/include -nostdsysteminc -triple thumbv7-apple-darwin9    -target-abi aapcs    -target-cpu cortex-a8    -mfloat-abi hard    -ffreestanding    -emit-llvm -w -o - /home/morbo/llvm/llvm-project/clang/test/CodeGen/arm-aapcs-vfp.c
[10/10] Creating executable symlink bin/clang
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: /home/morbo/llvm/llvm-project/build.opt/bin/clang -cc1 -internal-isystem /home/morbo/llvm/llvm-project/build.opt/lib/clang/18/include -nostdsysteminc -triple thumbv7-apple-darwin9 -target-abi aapcs -target-cpu cortex-a8 -mfloat-abi hard -ffreestanding -emit-llvm -w -o - /home/morbo/llvm/llvm-project/clang/test/CodeGen/arm-aapcs-vfp.c
1.	<eof> parser at end of file
2.	Optimizer
 #0 0x00005571cee481c8 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x360f1c8)
 #1 0x00005571cee45d6e llvm::sys::RunSignalHandlers() (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x360cd6e)
 #2 0x00005571cee48878 SignalHandler(int) Signals.cpp:0:0
 #3 0x00007f474f036510 (/lib/x86_64-linux-gnu/libc.so.6+0x3c510)
 #4 0x00005571ce5990f5 llvm::TargetLoweringBase::findRepresentativeClass(llvm::TargetRegisterInfo const*, llvm::MVT) const (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x2d600f5)
 #5 0x00005571ce59a131 llvm::TargetLoweringBase::computeRegisterProperties(llvm::TargetRegisterInfo const*) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x2d61131)
 #6 0x00005571cd18eac5 llvm::ARMTargetLowering::ARMTargetLowering(llvm::TargetMachine const&, llvm::ARMSubtarget const&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x1955ac5)
 #7 0x00005571cd0dfbb1 llvm::ARMSubtarget::ARMSubtarget(llvm::Triple const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, llvm::ARMBaseTargetMachine const&, bool, bool) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x18a6bb1)
 #8 0x00005571cd0e255a llvm::ARMBaseTargetMachine::getSubtargetImpl(llvm::Function const&) const (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x18a955a)
 #9 0x00005571cd0e2691 llvm::ARMBaseTargetMachine::getTargetTransformInfo(llvm::Function const&) const (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x18a9691)
#10 0x00005571d00187bd std::_Function_handler<llvm::TargetTransformInfo (llvm::Function const&), llvm::TargetMachine::getTargetIRAnalysis() const::$_0>::_M_invoke(std::_Any_data const&, llvm::Function const&) TargetMachine.cpp:0:0
#11 0x00005571ce16747e llvm::TargetIRAnalysis::run(llvm::Function const&, llvm::AnalysisManager<llvm::Function>&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x292e47e)
#12 0x00005571cff6e35f llvm::detail::AnalysisPassModel<llvm::Function, llvm::TargetIRAnalysis, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x473535f)
#13 0x00005571ce8c5cd5 llvm::AnalysisManager<llvm::Function>::getResultImpl(llvm::AnalysisKey*, llvm::Function&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x308ccd5)
#14 0x00005571cdf1a0de llvm::AssumptionAnalysis::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x26e10de)
#15 0x00005571cff699d0 llvm::detail::AnalysisPassModel<llvm::Function, llvm::AssumptionAnalysis, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x47309d0)
#16 0x00005571ce8c5cd5 llvm::AnalysisManager<llvm::Function>::getResultImpl(llvm::AnalysisKey*, llvm::Function&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x308ccd5)
#17 0x00005571cdf253a0 llvm::BasicAA::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x26ec3a0)
#18 0x00005571cff6e7d7 llvm::detail::AnalysisPassModel<llvm::Function, llvm::BasicAA, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x47357d7)
#19 0x00005571ce8c5cd5 llvm::AnalysisManager<llvm::Function>::getResultImpl(llvm::AnalysisKey*, llvm::Function&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x308ccd5)
#20 0x00005571cff9e05c void llvm::AAManager::getFunctionAAResultImpl<llvm::BasicAA>(llvm::Function&, llvm::AnalysisManager<llvm::Function>&, llvm::AAResults&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x476505c)
#21 0x00005571cdf11b3d llvm::AAManager::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x26d8b3d)
#22 0x00005571cfef09df llvm::detail::AnalysisPassModel<llvm::Function, llvm::AAManager, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function>::Invalidator>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x46b79df)
#23 0x00005571ce8c5cd5 llvm::AnalysisManager<llvm::Function>::getResultImpl(llvm::AnalysisKey*, llvm::Function&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x308ccd5)
#24 0x00005571ce9200e8 llvm::AAResults& llvm::function_ref<llvm::AAResults& (llvm::Function&)>::callback_fn<llvm::AlwaysInlinerPass::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&)::$_2>(long, llvm::Function&) AlwaysInliner.cpp:0:0
#25 0x00005571ce91e6f7 (anonymous namespace)::AlwaysInlineImpl(llvm::Module&, bool, llvm::ProfileSummaryInfo&, llvm::function_ref<llvm::AssumptionCache& (llvm::Function&)>, llvm::function_ref<llvm::AAResults& (llvm::Function&)>, llvm::function_ref<llvm::BlockFrequencyInfo& (llvm::Function&)>) AlwaysInliner.cpp:0:0
#26 0x00005571ce91e155 llvm::AlwaysInlinerPass::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x30e5155)
#27 0x00005571cff752cd llvm::detail::PassModel<llvm::Module, llvm::AlwaysInlinerPass, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x473c2cd)
#28 0x00005571ce8c2196 llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x3089196)
#29 0x00005571cf04734f (anonymous namespace)::EmitAssemblyHelper::RunOptimizationPipeline(clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream>>&, std::unique_ptr<llvm::ToolOutputFile, std::default_delete<llvm::ToolOutputFile>>&, clang::BackendConsumer*) BackendUtil.cpp:0:0
#30 0x00005571cf03d51c clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::StringRef, llvm::Module*, clang::BackendAction, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream>>, clang::BackendConsumer*) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x380451c)
#31 0x00005571cf5952a6 clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x3d5c2a6)
#32 0x00005571d0f3c066 clang::ParseAST(clang::Sema&, bool, bool) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x5703066)
#33 0x00005571cf819aaf clang::FrontendAction::Execute() (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x3fe0aaf)
#34 0x00005571cf789f4d clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x3f50f4d)
#35 0x00005571cf8f1147 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x40b8147)
#36 0x00005571cc9142a6 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x10db2a6)
#37 0x00005571cc91076e ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&, llvm::ToolContext const&) driver.cpp:0:0
#38 0x00005571cc90f456 clang_main(int, char**, llvm::ToolContext const&) (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x10d6456)
#39 0x00005571cc920331 main (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x10e7331)
#40 0x00007f474f0216ca __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:74:3
#41 0x00007f474f021785 call_init ./csu/../csu/libc-start.c:128:20
#42 0x00007f474f021785 __libc_start_main ./csu/../csu/libc-start.c:347:5
#43 0x00005571cc90cbf1 _start (/home/morbo/llvm/llvm-project/build.opt/bin/clang+0x10d3bf1)
Segmentation fault

@nickdesaulniers
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this was reverted in 42204c9.

$ cd llvm-project
$ git checkout 23c47eba8797
$ cd llvm/build && ninja clang && cd -
$ llvm-lit -vv clang/test/CodeGen/arm-aapcs-vfp.c
llvm-lit: /android0/llvm-project/llvm/utils/lit/lit/llvm/config.py:488: note: using clang: /android0/llvm-project/llvm/build/bin/clang
-- Testing: 1 tests, 1 workers --
PASS: Clang :: CodeGen/arm-aapcs-vfp.c (1 of 1)

Testing Time: 0.46s

Total Discovered Tests: 1
  Passed: 1 (100.00%)

The test passes for me BEFORE your revert.

The stack trace you posted doesn't reference code I added; even if inlining were the case I don't see how TargetLoweringBase::findRepresentativeClass were to call into anything I added or changed.

Wouldn't the presubmit or postsubmit tests have caught this if it was truly something that I regressed? That only you have been able to reproduce this far makes me curious about your environment, not my patch.

@nickdesaulniers
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bwendling
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could have been an unrelated patch that caused it and your patch tickled it just enough to make it fail consistently. I ran bisect on my branch and this was the first patch that caused the failure.

Please sign in to comment.