Skip to content

-Xarch_device fails to propagate -O1 #110325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Artem-B opened this issue Sep 27, 2024 · 1 comment · Fixed by #126101
Closed

-Xarch_device fails to propagate -O1 #110325

Artem-B opened this issue Sep 27, 2024 · 1 comment · Fixed by #126101
Assignees
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl'

Comments

@Artem-B
Copy link
Member

Artem-B commented Sep 27, 2024

Clang reports an error if one attempts to pass -O1 to a CUDA/HIP sib-compilation via -Xarch_device:

bin/clang++ -x cuda /dev/null --offload-arch=sm_60 -Xarch_device -O1 -c -o /tmp/foo.o -nocudainc -nocudalib
clang++: error: invalid Xarch argument: '-Xarch_device -O1', not all driver options can be forwared via Xarch argument
clang++: error: invalid Xarch argument: '-Xarch_device -O1', not all driver options can be forwared via Xarch argument

It appears to be a bug. The code that handles -Xarch.. options appears to misinterpret -O1 as a clang-cl option "/O":

} else if (XarchArg->getOption().hasFlag(options::NoXarchOption)) {

(gdb) p *XarchArg->getOption().Info
$41 = {
  Prefixes = llvm::ArrayRef of length 2 = {
    {
      <llvm::StringRef> = "/", <No data fields>},
    {
      <llvm::StringRef> = "-", <No data fields>}
  },
  PrefixedName = {
    <llvm::StringRef> = "/O", <No data fields>},
  HelpText = 0x55fb406ce5d4 "Set multiple /O flags at once; e.g. '/O2y-' for '/O2 /Oy-'",
  HelpTextsForVariants = {
    _M_elems = {
      {
        first = {
          _M_elems = {
            0,
            0
          }
        },
        second = 0x0
      }
    }
  },
  MetaVar = 0x55fb402591a2 "<flags>",
  ID = 3005,
  Kind = 4 '\004',
  Param = 0 '\000',
  Flags = 16,
  Visibility = 66,
  GroupID = 20,
  AliasID = 0,
  AliasArgs = 0x0,
  Values = 0x0
}

This is the option here:

def _SLASH_O : CLJoined<"O", [CLOption, DXCOption]>,

The option has NoXarchOption flag set, which triggers the error.

The correct option should've been this one:

def O : Joined<["-"], "O">, Group<O_Group>,

@Artem-B Artem-B self-assigned this Sep 27, 2024
@EugeneZelenko EugeneZelenko added clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' and removed new issue labels Sep 27, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 27, 2024

@llvm/issue-subscribers-clang-driver

Author: Artem Belevich (Artem-B)

Clang reports an error if one attempts to pass `-O1` to a CUDA/HIP sib-compilation via `-Xarch_device`:
bin/clang++ -x cuda /dev/null --offload-arch=sm_60 -Xarch_device -O1 -c -o /tmp/foo.o -nocudainc -nocudalib
clang++: error: invalid Xarch argument: '-Xarch_device -O1', not all driver options can be forwared via Xarch argument
clang++: error: invalid Xarch argument: '-Xarch_device -O1', not all driver options can be forwared via Xarch argument

It appears to be a bug. The code that handles -Xarch.. options appears to misinterpret -O1 as a clang-cl option "/O":

} else if (XarchArg->getOption().hasFlag(options::NoXarchOption)) {

(gdb) p *XarchArg-&gt;getOption().Info
$41 = {
  Prefixes = llvm::ArrayRef of length 2 = {
    {
      &lt;llvm::StringRef&gt; = "/", &lt;No data fields&gt;},
    {
      &lt;llvm::StringRef&gt; = "-", &lt;No data fields&gt;}
  },
  PrefixedName = {
    &lt;llvm::StringRef&gt; = "/O", &lt;No data fields&gt;},
  HelpText = 0x55fb406ce5d4 "Set multiple /O flags at once; e.g. '/O2y-' for '/O2 /Oy-'",
  HelpTextsForVariants = {
    _M_elems = {
      {
        first = {
          _M_elems = {
            0,
            0
          }
        },
        second = 0x0
      }
    }
  },
  MetaVar = 0x55fb402591a2 "&lt;flags&gt;",
  ID = 3005,
  Kind = 4 '\004',
  Param = 0 '\000',
  Flags = 16,
  Visibility = 66,
  GroupID = 20,
  AliasID = 0,
  AliasArgs = 0x0,
  Values = 0x0
}

This is the option here:

def _SLASH_O : CLJoined<"O", [CLOption, DXCOption]>,

The option has NoXarchOption flag set, which triggers the error.

The correct option should've been this one:

def O : Joined<["-"], "O">, Group<O_Group>,

jhuber6 added a commit to jhuber6/llvm-project that referenced this issue Feb 6, 2025
Summary:
Currently the `-Xarch` argument needs to re-parse the option, which goes
through every single registered argument. This causes errors when trying
to pass `-O1` through it because it thinks it's a DXC option. This patch
changes the behavior to only allow `clang` options. Concievably we could
detect the driver mode to make this more robust, but I don't know if
there are other users for this.

Fixes: llvm#110325
@jhuber6 jhuber6 closed this as completed in fe58eee Feb 6, 2025
github-actions bot pushed a commit to arm/arm-toolchain that referenced this issue Feb 6, 2025
Summary:
Currently the `-Xarch` argument needs to re-parse the option, which goes
through every single registered argument. This causes errors when trying
to pass `-O1` through it because it thinks it's a DXC option. This patch
changes the behavior to only allow `clang` options. Concievably we could
detect the driver mode to make this more robust, but I don't know if
there are other users for this.

Fixes: llvm/llvm-project#110325
Icohedron pushed a commit to Icohedron/llvm-project that referenced this issue Feb 11, 2025
Summary:
Currently the `-Xarch` argument needs to re-parse the option, which goes
through every single registered argument. This causes errors when trying
to pass `-O1` through it because it thinks it's a DXC option. This patch
changes the behavior to only allow `clang` options. Concievably we could
detect the driver mode to make this more robust, but I don't know if
there are other users for this.

Fixes: llvm#110325
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl'
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants