[PATCH] D92097: [RISCV] Basic jump table lowering

Jessica Clarke via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 26 07:19:50 PST 2020

jrtc27 added inline comments.

Comment at: llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp:126
-  BuildMI(NewMBB, DL, TII->get(RISCV::AUIPC), DestReg)
-      .addDisp(Symbol, 0, FlagsHi);
+  if (Symbol.isJTI()) {
+    BuildMI(NewMBB, DL, TII->get(RISCV::AUIPC), DestReg)
jrtc27 wrote:
> nand wrote:
> > jrtc27 wrote:
> > > nand wrote:
> > > > jrtc27 wrote:
> > > > > Why is it that expandAuipcInstPair needs to handle this specially but there's no corresponding logic for medlow's plain LUI/ADDI?
> > > > Based on my understanding, `addDisp` seems to be a generic method that produces an operand with an arbitrary offset from some reference, however it doesn't seem like offsets into jump tables are supported. Since the offset is always 0, the jump table index is handled separately. I might be wrong though.
> > > Right, then surely the correct fix is to add an MO_JumpTableIndex case to addDisp that calls addJumpTableIndex?
> > `addDisp` is common to all architectures, so I am reluctant to change it. Furthermore, `addDisp` takes an offset operand and the only sensible value for jump tables is 0. I think handling jump tables separately is nicer than asserting in `addDisp` that `off` is 0.
> I disagree. It's a generic method meant to take any operand one would reasonably want to use. We already use it for the very-similar constant pool index. You also don't need to assert `off` is 0; that's not done for CPIs, and if you pass in non-zero then you get what you asked for, ie `Symbol.getIndex() + off`. The only reason it's not currently supported in `addDisp` is that nobody has yet needed it, as other backends tend to just duplicate lowering for the different hooks. Just mirror the CPI code for what is basically a special case of it.
Note that this has been done in the past for RISC-V too; see https://reviews.llvm.org/D60136 (needed for https://reviews.llvm.org/D54143).

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list