[PATCH] D118009: [ARM] Make getInstSizeInBytes() use instruction size from InstrInfo.td
Nick Desaulniers via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 24 12:13:53 PST 2022
nickdesaulniers added a comment.
Please appease the linter for the newly added tests.
Other reviewers should be careful with review here. My major concern that I have not yet verified is whether these pseudo's always expand to the same number of instructions or not, which would affect their corresponding size.
================
Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:759
// pseudo-instruction sizes are zero.
- return 0;
+ return MCID.getSize();
case TargetOpcode::BUNDLE:
----------------
so the Aarch64 version of this patch does the opposite. It checks the `getSize()` first, then returns early before the switch.
https://reviews.llvm.org/D117970
Is there a reason we can't or shouldn't be consistent between the two targets?
================
Comment at: llvm/unittests/Target/ARM/CMakeLists.txt:13
MC
+ MIRParser
SelectionDAG
----------------
should this be a separate change?
================
Comment at: llvm/utils/gn/secondary/llvm/unittests/Target/ARM/BUILD.gn:5
deps = [
+ "//llvm/lib/CodeGen/MIRParser",
"//llvm/lib/MC",
----------------
different change?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118009/new/
https://reviews.llvm.org/D118009
More information about the llvm-commits
mailing list