[lldb] [llvm] [lldb][RISCV] function calls support in lldb expressions (PR #99336)

[Craig Topper via llvm-commits]

topperc 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.

That code doesn't look right. Shouldn't it be using eRISCV_float_abi_mask. It's treat each single and double as bit positions, but they are really encodings in 2-bit field.

```
     if ((arch_flags & ArchSpec::eRISCV_float_abi_mask) ==
         ArchSpec::eRISCV_float_abi_single)
       features_str += "+f,";
     if ((arch_flags & ArchSpec::eRISCV_float_abi_mask) ==
         ArchSpec::eRISCV_float_abi_double)
       features_str += "+f,+d,";
```

https://github.com/llvm/llvm-project/pull/99336