[PATCH] D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k
Bruno Cardoso Lopes via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 19 11:45:53 PST 2020
bruno added inline comments.
================
Comment at: clang/include/clang/Driver/Options.td:3125
+foreach i = {0-4} in
+ def m680#i#0 : Flag<["-"], "m680"#i#"0">, Group<m_m68k_Features_Group>;
----------------
rengolin wrote:
> Same question as @RKSimon had below: Shouldn't this cover all models the back-end recognises?
Unless you are planning to add 100 or more target variations I'd prefer to see these explicitly defined instead of a `foreach`. If I'm grepping for a specific CPU variation in the code base it's nice to get that information easily.
================
Comment at: clang/lib/Driver/ToolChains/Arch/M68k.cpp:37
+ return CPU;
+ else
+ return "";
----------------
No need for this `else` here.
================
Comment at: clang/lib/Driver/ToolChains/Arch/M68k.cpp:51
+ }
+ return CPUName.str();
+ }
----------------
RKSimon wrote:
> Can't we just use StringSwitch here?
+1
================
Comment at: clang/lib/Driver/ToolChains/Arch/M68k.cpp:64
+ return "M68040";
+ }
+
----------------
RKSimon wrote:
> (style) remove unnecessary braces?
and also the unnecessary `else`s.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88394/new/
https://reviews.llvm.org/D88394
More information about the cfe-commits
mailing list