[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

This revision was automatically updated to reflect the committed changes. Closed by commit rG0e4827aa4e4a: [ARM][MachineOutliner] Add Machine Outliner support for ARM. (authored by yroux). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76066/new/ htt

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

yroux updated this revision to Diff 263830. yroux added a comment. I found a last issue when doing a full bootstrap with -moutline used to build clang, there was a case of thunk outlining where the original was call a `BLX LR` which was broken by the outlining call `BL OUTLINE_FUNCTION_X` I fix

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

efriedma accepted this revision. efriedma added a comment. LGTM with one minor comment Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5607 + // * Register R12(IP), + // * Condition codes (and thus the CPSR register) + // yroux wrote: > efriedma wrote:

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

yroux added a comment. In D76066#2022025 , @samparker wrote: > Ah, yes, good point. Then I have nothing else, but definitely worth waiting > to see if Eli has further comments. Yes I'll wait for Eli's feedback and update the other patches. Thanks for t

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

samparker accepted this revision. samparker added a comment. This revision is now accepted and ready to land. > Eli said in his comments we don't really need to worry about APCS but only to > linker veneers and I don't think a linker would clear the sticky Q bit or > touch the GE ones. Ah, yes,

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

efriedma added inline comments. Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5813 + // Don't touch the link register + if (MI.readsRegister(ARM::LR, TRI) || MI.modifiesRegister(ARM::LR, TRI)) +return outliner::InstrType::Illegal; yroux wrote: > efri

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

yroux updated this revision to Diff 262205. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76066/new/ https://reviews.llvm.org/D76066 Files: clang/lib/Driver/ToolChains/Clang.cpp llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp llvm/lib/Target/ARM/ARMB

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

yroux added a comment. In D76066#2014697 , @samparker wrote: > Thanks for adding the MVE changes, but I also still don't see any DSP tests, > i.e QADD, SADD16. hmm... I was wrong w/r to DSP instructions Q and GE flags handling, I don't get what I did i

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

efriedma added inline comments. Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5806 +if (Opc == ARM::BL || Opc == ARM::tBL || Opc == ARM::BLX || +Opc == ARM::tBLXr || Opc == ARM::tBLXi) + UnknownCallOutlineType = outliner::InstrType::LegalTerminator; --

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

efriedma added inline comments. Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5607 + // * Register R12(IP), + // * Condition codes (and thus the CPSR register) + // If you control all the instructions that execute, you don't need to worry about what th

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

samparker added a comment. Thanks for adding the MVE changes, but I also still don't see any DSP tests, i.e QADD, SADD16. Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5753 + // ahead and skip over them. + if (MI.isKill()) +return outliner::InstrType::Invisible; -

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

yroux updated this revision to Diff 261237. yroux added a comment. This update avoids outlining MVE instructions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76066/new/ https://reviews.llvm.org/D76066 Files: clang/lib/Driver/ToolChains/Clang.c

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

samparker added inline comments. Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5606 + // candidates. + auto CantGuaranteeValueAcrossCall = [&TRI](outliner::Candidate &C) { +// If the unsafe registers in this block are all dead, then we don't need yro

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

yroux added inline comments. Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5606 + // candidates. + auto CantGuaranteeValueAcrossCall = [&TRI](outliner::Candidate &C) { +// If the unsafe registers in this block are all dead, then we don't need sampark

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

samparker added inline comments. Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5606 + // candidates. + auto CantGuaranteeValueAcrossCall = [&TRI](outliner::Candidate &C) { +// If the unsafe registers in this block are all dead, then we don't need yro

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

yroux added inline comments. Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5606 + // candidates. + auto CantGuaranteeValueAcrossCall = [&TRI](outliner::Candidate &C) { +// If the unsafe registers in this block are all dead, then we don't need yroux w

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

yroux updated this revision to Diff 260921. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76066/new/ https://reviews.llvm.org/D76066 Files: clang/lib/Driver/ToolChains/Clang.cpp llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp llvm/lib/Target/ARM/ARMB

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

yroux updated this revision to Diff 260916. yroux edited the summary of this revision. yroux added a comment. Here is a new update of the patch. I remove the logic to disable LowOverheadLoops pass since Eli has added the live-ins infos inside outlined functions in D78605

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

yroux added inline comments. Comment at: llvm/lib/Target/ARM/ARMTargetMachine.cpp:553 addPass(createARMConstantIslandPass()); - addPass(createARMLowOverheadLoopsPass()); + if (!MachineOutlinerEnabled) +addPass(createARMLowOverheadLoopsPass()); samparker

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

samparker added inline comments. Comment at: llvm/lib/Target/ARM/ARMTargetMachine.cpp:553 addPass(createARMConstantIslandPass()); - addPass(createARMLowOverheadLoopsPass()); + if (!MachineOutlinerEnabled) +addPass(createARMLowOverheadLoopsPass()); yroux

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

yroux added a comment. Hi Sam, Thanks for your comments. Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5606 + // candidates. + auto CantGuaranteeValueAcrossCall = [&TRI](outliner::Candidate &C) { +// If the unsafe registers in this block are all dead, then we don'

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

samparker added a comment. Hi Yvan, Thanks for adding the tests, I've added a few concerns and questions. Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:5606 + // candidates. + auto CantGuaranteeValueAcrossCall = [&TRI](outliner::Candidate &C) { +// If the unsafe r

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

yroux updated this revision to Diff 254834. yroux added a comment. Here is an update of the patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76066/new/ https://reviews.llvm.org/D76066 Files: clang/lib/Driver/ToolChains/Clang.cpp llvm/includ

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

yroux marked 5 inline comments as done. yroux added a comment. Thanks for the review Sam, I'll update the patch with more tests (for CPSR/LR it was part of D76068 but you are right it should be done here) and take your comments into account. In D76066#1921331

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

samparker added a comment. Hi, I like reading your code! But I'm concerned around the lack of tests here, specifically: - CPSR liveness. - LR liveness. - All things PIC related. - Linkage legality. - A negative test for Thumb-1. - Inline asm generally concerns me too. Commen

[PATCH] D76066: [ARM][MachineOutliner] Add Machine Outliner support for ARM

yroux created this revision. yroux added reviewers: t.p.northover, efriedma, paquette, samparker, SjoerdMeijer. Herald added subscribers: llvm-commits, cfe-commits, danielkiss, hiraditya, kristof.beyls. Herald added projects: clang, LLVM. Enables Machine Outlining support on ARM for ARM and Thum