lenary marked an inline comment as done.
lenary added inline comments.

================
Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:475
+
+    if (MArch.startswith_lower("rv32")) {
+      if (MArch.substr(4).contains_lower("d") ||
----------------
rogfer01 wrote:
> `llvm::StringSwitch` has a method `StartsWithLower` which might help make the 
> logic a bit clearer
> 
> Something like this (I haven't tested it!)
> 
> ```lang=cpp
> StringRef ABIFromMarch = StringSwitch(MArch)
>    .StartsWithLower("rv32d", "ilp32d")
>    .StartsWithLower("rv32g", "ilp32d")
>    .StartsWithLower("rv32e", "ilp32e")
>    .StartsWithLower("rv32", "ilp32")
> 
>    .StartsWithLower("rv64d", "lp64d")
>    .StartsWithLower("rv64g", "lp64d")
>    .StartsWithLower("rv64", "lp64").
> 
>    .Default("");
> 
> if (!ABIFromMarch.empty()) return ABIFromMarch;
> ```
Sadly I don't think this will work, because of the case of matching `rv32*d*` 
and `rv64*d*` (the `March.substr(4).contains_lower("d")` cases) from 
config.gcc. Explicitly "d" does not come immediately after `rv<32/64>`, it can 
come anywhere after like in `rv32imafdc`.

The other issue I have with the StringSwitch is that it requires I have a 
default, which I feel makes the control flow harder to understand, rather than 
easier. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69383



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

Reply via email to