[PATCH] D88625: MCExpr::evaluateAsRelocatableImpl : allow evaluation of non-VK_None MCSymbolRefExpr when MCAsmLayout is available

[James Y Knight via Phabricator via llvm-commits]

jyknight added inline comments.


================
Comment at: llvm/lib/MC/MCExpr.cpp:811
               Res, Asm, Layout, Fixup, Addrs, InSet || IsMachO)) {
+        // Attach the original VariantKind to SymA of the result.
+        if (Layout && Res.getSymA()) {
----------------
Maybe to prevent the nonsensical result in the test below, have a condition, like:
```
        // If the reference has a variant kind, we can only handle expressions
        // which evaluate exactly to a single unadorned symbol.
        if (Kind != MCSymbolRefExpr::VK_None) {
          if (Res.getRefKind() != MCSymbolRefExpr::VK_None || !Res.getSymA() || Res.getSymB() || Res.getConstant())
            return false;
          Res = ...
        }
```


================
Comment at: llvm/test/MC/ARM/ehabi-personality-abs.s:12
 
-@ CHECK: R_ARM_NONE __aeabi_unwind_cpp_pr0
-
+@@ GNU as also emits a R_ARM_NONE, which is insignificant.
+@ CHECK:      Section {{.*}} .rel.ARM.exidx {
----------------
", because the target is an absolute symbol."


================
Comment at: llvm/test/MC/ELF/relocation-alias.s:18
+memcpy_plus_1 = __GI_memcpy+1
+call memcpy_plus_1 at PLT
+
----------------
This is compatible with what GNU as does, but is nonsensical behavior. Do you think we really want to support it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88625