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

================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:575
+    // times 100.
+    if (getTarget().getTargetOpts().CodeObjectVersion != "none") {
+      unsigned CodeObjVer;
----------------
tra wrote:
> yaxunl wrote:
> > tra wrote:
> > > When will it ever be set to `none`? Does the new option parser enforce 
> > > the default & version values specified in the tablegen?
> > > If so, then it should never be `none`.
> > > If the `Values` specified for the option are not enforced, then the 
> > > condition will be true if user specifies any invalid value other than 
> > > `none`.
> > > 
> > Normal HIP programs should only use `-mcode-object-version={2|3|4|5}`. 
> > clang driver enforces that.
> > 
> > ROCm device library need to be compiled with `-Xclang 
> > -mcode-object-version=none` so that the module flag is not emitted. Since 
> > this use case is not for common users,  `-mcode-object-version=none` can 
> > only be used with -cc1.
> I'm surprised that cc1 does not check option validity. It does check them at 
> the driver level: https://godbolt.org/z/9T6n47es9
> 
> Assuming that's intentional, and there are no checks in cc1, then we should 
> probably remove the now-useless `Values` from the option tablegen and add a 
> test to verify what happens with invalid values.
> 
> 
I will use MarshallingInfoEnum, which will check the values. Will add a test 
for invalid values.


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

https://reviews.llvm.org/D119026

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

Reply via email to