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

[Alex Bradbury via Phabricator via llvm-commits]

asb accepted this revision.
asb added a comment.

I added some very minor nits in comments, but other than those I think this is looking good to land. Thanks! It's also worth checking @rogfer01's suggestion before committing https://reviews.llvm.org/D55305#inline-553463



================
Comment at: lib/Target/RISCV/RISCVISelLowering.cpp:500
+    SDValue Addr = DAG.getTargetGlobalAddress(GV, DL, Ty, 0, 0);
+    SDValue Load = SDValue(DAG.getMachineNode(RISCV::PseudoLA_TLS_IE, DL, Ty, Addr), 0);
+
----------------
Line is longer than 80 chars, `git clang-format` will reformat niclely (assuming you have the git-clang-format helper on your PATH)


================
Comment at: lib/Target/RISCV/RISCVISelLowering.cpp:538
+  SDValue Addr = DAG.getTargetGlobalAddress(GV, DL, Ty, 0, 0);
+  SDValue Load = SDValue(DAG.getMachineNode(RISCV::PseudoLA_TLS_GD, DL, Ty, Addr), 0);
+
----------------
Line is longer than 80 chars, `git clang-format` will reformat niclely (assuming you have the git-clang-format helper on your PATH)


================
Comment at: lib/Target/RISCV/RISCVISelLowering.cpp:574
+  default:
+    report_fatal_error("Unsupported TLS model");
+  case TLSModel::LocalExec:
----------------
The enumeration is fully covered, so we shouldn't have the default case here: https://llvm.org/docs/CodingStandards.html#don-t-use-default-labels-in-fully-covered-switches-over-enumerations


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55305