[PATCH] D79625: [PowerPC] Extend .reloc directive on PowerPC

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp marked an inline comment as not done.
stefanp added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp:1811
+/// ParseDirectiveReloc
+/// Very similar to the one in AsmParser::parseDirectiveReloc except that we
+/// want to be able to parse MCExpr::Binary too.
----------------
stefanp wrote:
> sfertile wrote:
> > Was the problem that the existing functionality can only parse a constant or symbol reference for the offset?  If so why extend the functionality in the PPCASMParser as opposed to  `AsmParser::parseDirectiveReloc`? I'm not super familiar with the .reloc directive but gas documents accepting symbol + offset for the first expression. https://sourceware.org/binutils/docs/as/Reloc.html#Reloc
> This is a very good point: this is just an extension of existing functionality. I wanted to make it PPC specific because I believed that the community would more  easily accept this code as PPC specific. I have nothing against making this target independent.
> I would also like to hear form some of the other reviewers on this as to whether or not they believe that making this target independent would make more sense.
After further consideration, I think we should probably leave things separate for now. No other targets have needed these features yet and it would probably be better to implement them on PowerPC first and test them that way. After some of the bugs have been worked out we can propose to make a wider change that will work on all targets with ELF and have a second patch for that.


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

https://reviews.llvm.org/D79625