jasonmolenda wrote:
I read through the patch again cleanly, and I don't have any problems with the
PR at this point. The only thing I would note is that in `GetClangTargetABI`
we're constructing a string which indicates which ISA extensions are enabled
(that are relevant here) to return a string like `lp64f`, which is then used in
`SetupTargetOpts` to add the feature flags that should be enabled for clang.
We are doing the exact same thing in `DisassemblerLLVMC::DisassemblerLLVMC`,
```
if (triple.isRISCV()) {
uint32_t arch_flags = arch.GetFlags();
if (arch_flags & ArchSpec::eRISCV_rvc)
features_str += "+c,";
if (arch_flags & ArchSpec::eRISCV_rve)
features_str += "+e,";
if ((arch_flags & ArchSpec::eRISCV_float_abi_single) ==
ArchSpec::eRISCV_float_abi_single)
features_str += "+f,";
if ((arch_flags & ArchSpec::eRISCV_float_abi_double) ==
ArchSpec::eRISCV_float_abi_double)
features_str += "+f,+d,";
```
etc. It's a small bit of duplication, but I expect these will be at risk of
diverging if done separately. I wonder if our ArchSpec should have a method to
get the clang feature flags.
https://github.com/llvm/llvm-project/pull/99336
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
