[PATCH] D119026: [HIP] Emit amdgpu_code_object_version module flag
Yaxun Liu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 7 10:56:36 PST 2022
yaxunl marked 2 inline comments as done.
yaxunl added inline comments.
Comment at: clang/include/clang/Driver/Options.td:3445
def mcode_object_version_EQ : Joined<["-"], "mcode-object-version=">, Group<m_Group>,
- HelpText<"Specify code object ABI version. Defaults to 4. (AMDGPU only)">,
- MetaVarName<"<version>">, Values<"2,3,4,5">;
+ HelpText<"Specify code object ABI version. Allowed values are 2, 3, 4, and 5. Defaults to 4. (AMDGPU only)">,
> `none` is missing. I'm not sure if we need to enumerate all values -- the list will eventually grow unacceptably long.
> Does the option parser print allowed values on error? That would be sufficient, IMO.
> Up to you.
The help text is for clang driver, where 'none' is not allowed. 'none' is only allowed with clang -cc1.
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1166
CmdArgs.insert(CmdArgs.begin() + 1, "-mllvm");
+ // -cc1as does not need -mcode-object-version option.
+ if (!IsCC1As)
> Do we really need a special case for `-cc1as`? What happens if we do pass the option to it?
> If cc1as does need a special case handling, we may want to add a test case for that.
TargetOptions are for -cc1 only. when passed to -cc1as, they are treated as unknown options and cause error. will add a test for that.
CHANGES SINCE LAST ACTION
More information about the cfe-commits