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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to