[lld] r234009 - [ARM] Implementation of R_ARM_TARGET1 relocation

Leny Kholodov lkholodov at accesssoftek.com
Fri Apr 10 09:26:18 PDT 2015


Hi Simon,

I added the prefix because from my point of view --target1-rel didn't 
reflect the relation of this option with ARM architecture. However, I 
think your argumentation is stronger so I'll fix the implementation. 
Thank you for pointing to this issue.

Best regards,
Leny Kholodov

On 10.04.2015 18:58, Simon Atanasyan wrote:
> Hi,
>
> On Fri, Apr 3, 2015 at 3:03 PM, Leny Kholodov
> <lkholodov at accesssoftek.com> wrote:
>> +def grp_targetopts : OptionGroup<"opts">,
>> +     HelpText<"ARCH SPECIFIC OPTIONS">;
>> +def arm_target1_rel : Flag<["--"], "arm-target1-rel">,
>> +     Group<grp_targetopts>, HelpText<"Interpret R_ARM_TARGET1 as R_ARM_REL32">;
>> +def arm_target1_abs : Flag<["--"], "arm-target1-abs">,
>> +     Group<grp_targetopts>, HelpText<"Interpret R_ARM_TARGET1 as R_ARM_ABS32">;
> Is it really necessary to add the target name prefix to these options?
> That prevents LLD to be a drop in replacement for GNU (bfd/gold)
> linkers.
>
> When we handle target specific options in the `GnuLdDriver` we know
> selected target and can process these options accordingly. To separate
> say arm specific options in the GnuLdOptions.td we can introduce new
> options group:
>
> ```
> def grp_arm_targetopts : OptionGroup<"ARM SPECIFIC OPTIONS">,
>                           Group<grp_targetopts>;
> def target1_rel : Flag<["--"], "target1-rel">,
>                    Group<grp_arm_targetopts>,
>                    HelpText<"Interpret R_ARM_TARGET1 as R_ARM_REL32">;
> def target1_abs : Flag<["--"], "target1-abs">,
>                    Group<grp_arm_targetopts>,
>                    HelpText<"Interpret R_ARM_TARGET1 as R_ARM_ABS32">;
> ```
>
> The only problem is that `llvm::OptTable::PrintHelp()` does not take
> in account sub-groups and does not print their names. But I think it
> is better to fix this problem.
>
> By the way, the Clang driver does not add any target specific prefix
> to option names.
>




More information about the llvm-commits mailing list