[PATCH] D69383: [RISCV] Match GCC `-march`/`-mabi` driver defaults

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 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") ||
----------------
lenary wrote:
> 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. 
Oh I see.

Then I would comment what this part does with a bit more detail right after the `// 2. Choose a default based on -march=`. For example

```
// rv32g | rv32*d -> ilp32d
// rv32e -> ilp32e
// rv32* -> ilp32
// rv64g | rv64*d -> lp64d
// rv64* -> lp64
```

Given that gcc is using a shell glob, then `GlobPattern` in `Support/GlobPattern.h` may help as well. But it might be overkill in this scenario.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69383