[PATCH] D55305: [RISCV] Add lowering of global TLS addresses

[Lewis Revill via Phabricator via llvm-commits]

lewis-revill added inline comments.


================
Comment at: lib/Target/RISCV/RISCVISelLowering.cpp:445
+    SDValue Addr = DAG.getTargetGlobalAddress(GV, DL, Ty, 0, 0);
+    SDValue Load = SDValue(DAG.getMachineNode(RISCV::PseudoLA_TLS_IE, DL, Ty, Addr), 0);
+
----------------
rogfer01 wrote:
> Because we run `BranchRelaxation` in `addPreEmitPass` but the expansion of these pseudos happens in `addPreEmitPass2` I think we need to update `RISCVInstrInfo::getInstSizeInBytes` to state that `PseudoLA_TLS_IE` and `PseudoLA_TLS_GD` will take 8 bytes (otherwise we run into "fixup errors" because the relaxation has been done using an underestimation).
> 
> (Alternatively we could use the `Size` attribute in tablegen but currently only `PseudoLI` does this and we don't use that one in codegen, so that is probably OK)
> 
> Also I wonder if the attribute `isAsmParserOnly = 1` should be updated to `0` so we are more honest with the current usage of these pseudos. That said from a cursory check in tablegen, that attribute does not seem very important at the moment.
Thanks, I'll make that change, I did the same for the PIC `PseudoLA` so I must have forgotten it for this.

The `isAsmParser` attribute seems to enforce that the pseudo instruction doesn't cross the barrier between CodeGen and MC. Might be worth removing it to prevent confusion, I could do that in a later patch since there are now a couple of pseudos like this.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55305