kito-cheng marked 5 inline comments as done.
kito-cheng added a comment.
Here is another solution for flexible multi-lib configuration I consider before:
- Add an option called `-fmultilib-config=`
- Using same or similar syntax with GCC's `--with-multilib-generator`
But the problem is it's really toooooo long for describe a multi-lib config.
e.g: current default bare-metal multi-lib configuration:
`--with-multilib-generator="rv32i-ilp32--c;rv32im-ilp32--c;rv32iac-ilp32--;rv32imac-ilp32--;rv32imafc-ilp32f-rv32imafdc-;rv64imc-lp64--;rv64imfc-lp64f--;rv64imac-lp64--;rv64imafdc-lp64d--"`
================
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1625
+ const std::string &MultilibOutput,
+ DetectedMultilibs &Result) {
+ llvm::StringSet<> AllABI;
----------------
khchen wrote:
> bool -> static bool?
> const std::string &MultilibOutput -> StringRef ?
>
> The function name seem like it's not a target-specific function, but it's
> only support RISC-V now. Maybe we could rename it or make an assertion in
> function to check the target should be RISC-V?
>
> Maybe we could rename it or make an assertion in function to check the target
> should be RISC-V?
Good point, renamed.
================
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1642
+ std::string CurrentArchOpt = Twine("march=", MArch).str();
+ std::string CurrentMCmodelOpt = llvm::StringSwitch<const char *>(CodeModel)
+ .Case("medium", "mcmodel=medany")
----------------
khchen wrote:
> Could they be declared as StringRef and use llvm::StringSwitch<StringRef>?
I would prefer keep `const char *` to prevent create a temporal object for
`StringRef` here.
And I also found another place are used same way:
```
$ grep "StringSwitch<const" * -R
clang/lib/Basic/SourceManager.cpp: llvm::StringSwitch<const char *>(BufStr)
clang/lib/Driver/ToolChains/Gnu.cpp: std::string CurrentMCmodelOpt =
llvm::StringSwitch<const char *>(CodeModel)
clang/lib/Driver/ToolChains/Clang.cpp: MipsTargetFeature =
llvm::StringSwitch<const char *>(Value)
clang/lib/Driver/ToolChains/Cuda.cpp: OOpt = llvm::StringSwitch<const char
*>(A->getValue())
clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp: OOpt =
llvm::StringSwitch<const char *>(A->getValue())
clang/lib/Driver/ToolChains/Arch/Mips.cpp: ABIName =
llvm::StringSwitch<const char *>(CPUName)
clang/lib/Driver/ToolChains/Arch/Mips.cpp: CPUName =
llvm::StringSwitch<const char *>(ABIName)
clang/lib/Driver/ToolChains/Arch/PPC.cpp: return llvm::StringSwitch<const
char *>(CPUName)
clang/lib/Driver/ToolChains/Arch/PPC.cpp: return llvm::StringSwitch<const char
*>(Name)
clang/lib/Driver/ToolChains/Arch/Sparc.cpp: return llvm::StringSwitch<const
char *>(Name)
clang/lib/Driver/ToolChains/Arch/Sparc.cpp: return llvm::StringSwitch<const
char *>(Name)
clang/lib/Driver/ToolChains/Darwin.cpp: return llvm::StringSwitch<const char
*>(Arch)
clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp: StringSwitch<const
char *>(T.first->getValueAsString("AccessQualifier"))
clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp: OS <<
StringSwitch<const char *>(
llvm/include/llvm/Support/FormatProviders.h: Stream << StringSwitch<const
char *>(Style)
llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp: StringSwitch<const
char *>(BroadcastString)
llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp: const char *Repl =
StringSwitch<const char *>(Name)
llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp: const char *Repl =
StringSwitch<const char *>(Op.getToken())
llvm/lib/Support/Host.cpp: return StringSwitch<const char
*>(StringRef(CPUStart, CPULen))
llvm/lib/Support/Host.cpp: return StringSwitch<const char *>(Part)
llvm/lib/Support/Host.cpp: return StringSwitch<const char *>(Part)
llvm/lib/Support/Host.cpp: return StringSwitch<const char *>(Part)
llvm/lib/Support/Host.cpp: return StringSwitch<const char *>(Part)
llvm/lib/Support/Host.cpp: return StringSwitch<const char *>(Part)
llvm/lib/Support/Host.cpp: return StringSwitch<const char *>(Part)
llvm/unittests/Support/Host.cpp: StringRef MCPU = StringSwitch<const char
*>(CPU)
```
================
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1650
+
+ if (File) {
+ SmallVector<StringRef, 128> Lines;
----------------
khchen wrote:
> ```
> if (!File)
> return;
> ```
Good suggestion, thanks!
================
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1703
+ }
+ }
+ Ms.emplace_back(Multilib);
----------------
Jim wrote:
> Do you have plan to support other kind of options to build multilib?
Currently, no, but I think could be consider in future.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97916/new/
https://reviews.llvm.org/D97916
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
