Skip to content
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

Work around xl compiler bug when nvcc preprocesses this file #2190

Merged
merged 1 commit into from
Mar 27, 2021
Merged

Work around xl compiler bug when nvcc preprocesses this file #2190

merged 1 commit into from
Mar 27, 2021

Conversation

white238
Copy link
Contributor

I agree that my contributions are licensed under the {fmt} license, and agree to future changes to the licensing.

This works around a compiler bug in xlc when the file is preprocessed by nvcc. This manifests as format_arg_store::desc being undefined at link time. Pulling out the detail::encode_types<Context, Args...>() into it's own line was the fix.

If you prefer to have this fixed in another way, I am open to suggestions.

More information here:
LLNL/axom#492

Write up about the minimal compiler reproducer by @joshessman-llnl:

So I was initially just looking to how minimally the behavior we were seeing with fmt could be reproduced, and was able to get to this:

constexpr int get_bar()
{
  return 0;
}

struct Foo
{
  // static constexpr int bar = 0; // Works with both foo.bar and Foo::bar
  static constexpr int bar = get_bar(); // Fails with foo.bar
};

int main()
{
  constexpr int y = Foo::bar; // Works just fine with get_bar()
  Foo foo;
  constexpr int x = foo.bar; // Fails with get_bar() - undefined reference to Foo::bar
  return 0;
}

The static member access through the operator. is a little unconventional but it's how fmt uses the desc variable that was modified in this PR.

I was still curious as to why it only failed with NVCC + XLC with -x cu and not with just XLC or NVCC + XLC not compiled as CUDA, so I started digging into the intermediate files. It looks like the CUDA C++ frontend ( cudafec++ ) parenthesizes the foo.bar:

constexpr int x = (foo.bar);

and this is what triggers the XLC bug. The parentheses are not meaningless here, I believe the type of foo.bar is int while the type of ( foo.bar ) is int& - it's possible that the reference-ness is the cause of the bug. I would consider NVCC's changing of int -> int& to also be a bug or at least unexpected, though clang++ is able to handle this.

After some further investigation it looks like XLC (run standalone, not through NVCC) only fails to link the cudafe++ -parenthesized version when debug symbols are enabled ( -g ) and with zero optimizations enabled ( -O0 )

In summary I think the XLC bug requires the following;

  • Initializing of a constexpr static member with the result a constexpr function instead of a literal
  • Access to that constexpr static member via an instance of the class instead of X::s
  • Access via class instance in a parenthesized expression
  • Debug symbols and no optimizations

@vitaut
Copy link
Contributor

vitaut commented Mar 25, 2021

Thanks for the PR. I suggest replacing store.desc with format_arg_store<Context, Args...>::desc in

: basic_format_args(store.desc, store.data_.args()) {}

instead of splitting the definition of desc.

@white238
Copy link
Contributor Author

Verified this worked against our plethora of compiler combinations we support. Thanks for the better fix!

@vitaut vitaut merged commit 9b34681 into fmtlib:master Mar 27, 2021
@white238 white238 deleted the bugfix/white238/xlc_nvcc_compiler_bug branch March 28, 2021 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants