[PATCH] D72183: [ELF][PPC64] Add --lax-call-lacks-nop

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/ELF/InputSection.cpp:979-980
+        // runtime. Just ignore if --lax-call-lacks-nop is specified.
         if ((bufLoc + 8 > bufEnd || read32(bufLoc + 4) != 0x60000000) &&
-            rel.sym->file != file) {
+            !(config->laxCallLacksNop && rel.sym->file == file)) {
+          std::string hint;
----------------
MaskRay wrote:
> ruiu wrote:
> > Can this condition be reduced to
> > 
> >   !config->laxCallLacksNop && rel.sym->file != file &&
> >   (bufLoc + 8 > bufEnd || read32(bufLoc + 4) != 0x60000000)
> > 
> > ?
> It can't.
> 
> `rel.sym->file == file` is the case we want to ignore when --lax-call-lacks-nop is specified.
> 
> So we use `!(config->laxCallLacksNop && rel.sym->file == file)` here.
OK. Can you factor out `config->laxCallLacksNop && rel.sym->file == file` as a boolean variable?


================
Comment at: lld/ELF/Options.td:225
+    HelpText<"(PowerPC64) Don't error on b/bl not followed by nop, if the target is "
+             "defined in the same file. This is for compatibility with GCC<5.4 and GCC<6.4">;
+
----------------
MaskRay wrote:
> ruiu wrote:
> > ruiu wrote:
> > > Does this mean "GCC 5.4 or earlier AND GCC 6.4 or earlier"? Did you mean GCC 5.4 to 6.4?
> > I"m confused. Doesn't "GCC 5.4 or earlier AND GCC 6.4 or earlier" just mean "GCC 5.4 or earlier"?
> Each major version has a release branch. This was fixed in 7 (not released at that time), then back ported to the release branches of 5 and 6. So certain 5.* and 6.* releases do not include the patch.
How about something like "GCC 5.0-5.6 and GCC 6.0-6.4"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72183