[PATCH] D157565: [CodeGen] Add AArch64 behavior to existing MFS tests

Thu Aug 17 16:33:30 PDT 2023

dhoekwater marked an inline comment as done.
dhoekwater added inline comments.

================
Comment at: llvm/lib/CodeGen/BasicBlockSections.cpp:294
+        continue;
+      NopInstr.addOperand(
+          MachineOperand::CreateImm(Nop.getOperand(0).getImm()));
----------------
rahmanl wrote:
> dhoekwater wrote:
> > rahmanl wrote:
> > > rahmanl wrote:
> > > > Is this because of ARM? Maybe add a comment to explain the rationale.
> > > I see, this is target specific now. Although this code was target-specific before too, but we only wanted code for X86. Do you think we should implement insertNoop (https://llvm.org/doxygen/classllvm_1_1TargetInstrInfo.html#a4ee57d5d6295dfeb44f3b55301b20020) for X86 and AArch64 now?
> > Yes! That's exactly what I was looking for, but I couldn't find the function since I was looking for something like "addNop". Should we do that in this patch or should I make another?
> Right. Separate the entire `avoidZeroOffsetLandingPad` changes into its own patch if you can. You might get new reviewers when implement `TargetInstrInfo`.
Unfortunately, we need AArch64 tests to test the `avoidZeroOffsetLandingPad` changes, and we need the changes so that the tests don't crash on AArch64.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157565/new/

https://reviews.llvm.org/D157565