[PATCH] D142933: Add -print-multi-selection-flags argument

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

Thanks for the updates, changes look good to me. I think that we can get away without trying to break the floating point options into flags, at least for how floating point units are available in hardware today.



================
Comment at: clang/lib/Driver/ToolChain.cpp:173
 }
 
+std::vector<std::string>
----------------
michaelplatings wrote:
> peter.smith wrote:
> > IIUC this needs to contain the union of all command-line options used by all clients of the YAML but not any of the existing hard-coded Multilibs?
> > 
> > There is a possibility that this function could get large over time, with many Toolchain and Architecture specific flags? It is it worth delegating some of the work to the Toolchain, for example at the moment the BareMetal driver won't support the sanitizers so this could be processed by a Toolchain specific function (Fuchsia in this case?). There's likely to be a lot of options used by many Multilib implementations so there is still scope for putting stuff here. We would need to know what driver and architecture we are using here though.
> > 
> > If we had already constructed the MultilibSet it could be possible to filter the flags against what the MultilibSet actually accepts?
> > 
> > Is it worth making that clear, or have some way for the other non YAML users to call print_multi_selection_flags? Or is the option purely for developing/debugging multilib as I'd not expect the output of print_multi_section flags to be used by a user of clang. In that case a translation of what command-line flags influence multilib selection (ideally filtered) through the processed YAML file would be great.
> Correct, we don't yet need to cater for existing hard-coded Multilibs.
> 
> I did think about putting some argument processing in ToolChain subclasses but for now I'm inclined to say YAGNI. If it's needed later on that's an easy refactoring. With the current form, there's less likelihood of different ToolChain subclasses diverging in the style of attributes they emit.
> 
> MultilibSet can use a regex, and both matching and not matching can be significant, so I'm not sure it makes sense to filter against that.
> 
> `-print-multi-selection-flags-experimental` can be used completely independently of any YAML or whether the current ToolChain class actually supports multilib. `-print-multi-selection-flags-experimental` is intended primarily for developing/debugging multilib.
Thinking about Arm floating point modelling (not really relevant to the mechanism here). Certainly since the Cortex cores the instructions available to an FPU are tied to the architecture. I.e. you cannot mix and match a FPU from an earlier or later architecture revision in practice, even if you can put it on the command-line in theory. I think this means that in the majority of cases we just need to match the floating point libraries with the architecture. The exceptions are the floating point variants with cut-down functionality:
* single-precision only (a nightmare of hard-float for floats and soft-float for doubles)
* d16 (only 16 double precision registers available rather than 32. I would expect this to be interface compatible with code compiled with 32-double precision registers as the calling convention doesn't change). All single-precision only are also d16.

I think these fpu options are restricted to Cortex-M and Cortex-R.

For a small number of targets we'd likely need at least two (single only, and d16) and at most 3 (single only, d16, and full) floating point variants. We'd need to map the fpu name suitable for the architecture but I don't think we would want to work too hard trying to map an arbitrary floating point unit that there is no available hardware for. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142933