[PATCH] D104473: RISCV: clean up target expression handling

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp:95
                                             const MCFixup *Fixup) const {
-  bool IsSymbolicDifference = false;
-  if (const auto *MBE = dyn_cast<MCBinaryExpr>(getSubExpr())) {
-    if (isa<MCBinaryExpr>(MBE->getLHS()) && isa<MCConstantExpr>(MBE->getRHS()))
-      MBE = cast<MCBinaryExpr>(MBE->getLHS());
-    IsSymbolicDifference = isa<MCSymbolRefExpr>(MBE->getLHS()) &&
-                           isa<MCSymbolRefExpr>(MBE->getRHS());
-  }
+  if (!getSubExpr()->evaluateAsRelocatable(Res, nullptr, nullptr))
+    return false;
----------------
IIRC passing `Layout` can fold more expressions. In this case is `Layout` harmful?


================
Comment at: llvm/test/MC/RISCV/expressions.s:7
+	lui a0, %hi(tls+0-.Ltmp1)
+# CHECK: :[[#@LINE-1]]:[[#]]: error: expected relocatable expression
+	lw a0, %lo(tls+0-.Ltmp1)(t0)
----------------
Better to expand `:[[#]]:` to an integer. When testing parsing errors, it is useful to have precise column  numbers.
Users need such information so it should be properly tested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104473