[PATCH] D48112: Simplify PPC64::calcEFlags().

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: lld/ELF/Arch/PPC64.cpp:121
+    if (Flag != 0 && Flag != 2)
+      error(toString(F) + ": ABI version " + Twine(Flag) + " is not supported");
   }
----------------
ruiu wrote:
> sfertile wrote:
> > I think we should still emit 2 separate error messages:
> > 
> > 1) if flags & 1 is set then emit an error message that reflects we don't support the V1 abi.
> > 2) if any flags bits other then bits 1 or 2 are set then we emit a message for unrecognized e_flags.
> What do you suggest for the second message? I thought it would be something like "unsupported ABI version: 3" which is basically the same error message for v1.
Maybe I am misunderstanding exactly what the eflags can be used for but I thought they can specify features/extensions as well as the abi versions. Right now only bits 0 and 1 are used to specify the abi, so if bit 1 is set we recognize that the user is giving us abi 1 input which isn't supported and let the user know exactly what the problem is.

If any bits above 1 are set we don't know if they are supposed to represent  a new abi flag, a new feature flag or if the input is corrupted, and in that case a message like `"Unrecognized feature flags: " + Twine (Flags & ~3)`  is likely to be more helpful. 


https://reviews.llvm.org/D48112