[PATCH] D46423: [WIP, RISCV] Support .option relax and .option norelax

Simon Cook via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 22 09:38:31 PDT 2018


simoncook updated this revision to Diff 148023.
simoncook retitled this revision from "[RISCV] Support .option relax and .option norelax" to "[WIP, RISCV] Support .option relax and .option norelax".
simoncook added a comment.

//Note: I've marked this as WIP whilst discussing the interface, I'm not suggesting hardcoding in a setSTI() function to MCAsmBackend, that will be removed from the final patch//

I added a test to check the relax feature affects symbol differences as we would expect, and it didn't have the intended effect. The issue is that the MCAsmBackend is created with a STI that is created before we start parsing assembly, and that when we parse .option relax/norelax/rvc/norvc, we create a new STI, but this remains local to the AsmParser, but the MCAsmBackend keeps the original. To compare, what happens for instructions, our MatchInstructionImpl is given the current STI, so has the default flags, so enabling/disable works as expected.

What I've done here is when we parse relax/norelax I get a handle on the AsmBackend and update a pointer to the current STI that should be used. (Currently this is clumsily pushed into the AsmBackend interface, if we go down this route I'll make this a target specific hook). This way when we go to emit the constant we know whether the relax flag is set.

This solves half the issue, the other is the issue about only having the last state available (described https://reviews.llvm.org/D45181#1086541), as expressions will be converted to relocation once the entire file is parsed, and there is nowhere for extra metadata that a particular expression should be relaxed.

I've given it some thought, and I think if we want to have these relocations be optional, then I think storing a flag in AsmBackend to switch on the relocations for all symbol differences makes more sense than the broken state if things are switched off. So setting `.option relax` would set this flag, but `.option norelax` wouldn't unset it, and the logic for `requiresDiffExpressionRelocations` would check STI and this flag, and if either were set this would return true. I think this seems a better user experience than having a warning, or letting this just break.

Any thoughts?


Repository:
  rL LLVM

https://reviews.llvm.org/D46423

Files:
  include/llvm/MC/MCAsmBackend.h
  lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
  lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
  lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h
  lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
  lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h
  test/MC/RISCV/option-invalid.s
  test/MC/RISCV/option-relax.s

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46423.148023.patch
Type: text/x-patch
Size: 7961 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180522/5a329228/attachment.bin>


More information about the llvm-commits mailing list