[PATCH] D123264: [RISCV] Pre-RA expand pseudos pass

Luís Marques via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 15 01:47:13 PDT 2022


luismarques added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/MachineInstr.h:1212
+    return isLabel() || isCFIInstruction() || getPreInstrSymbol() ||
+           getPostInstrSymbol();
+  }
----------------
HsiangKai wrote:
> Is it enough to only add checking of getPreInstrSymbol()?
What do you mean, enough for what? (This patch also checks `getPostInstrSymbol()`).
In my testing that was enough to ensure that the label always pointed to the correct upper instruction, but feedback about this is welcome.


================
Comment at: llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp:401
+                              MachineBasicBlock::iterator &NextMBBI);
+  bool expandLoadAddress(MachineBasicBlock &MBB,
+                         MachineBasicBlock::iterator MBBI,
----------------
HsiangKai wrote:
> redundant?
Thanks. I'll remove it in the next revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123264



More information about the llvm-commits mailing list