Page MenuHomePhabricator

[NVPTX] Let there be One True Way to set NVVMReflect params.
ClosedPublic

Authored by jlebar on Jan 13 2017, 2:00 PM.

Details

Summary

Previously there were three ways to inform the NVVMReflect pass whether
you wanted to flush denormals to zero:

  • An LLVM command-line option
  • Parameters to the NVVMReflect constructor
  • Metadata on the module itself.

This change removes the first two, leaving only the third.

The motivation for this change, aside from simplifying things, is that
we want LLVM to be aware of whether it's operating in FTZ mode, so other
passes can use this information. Ideally we'd have a target-generic
piece of metadata on the module. This change moves us in that
direction.

Diff Detail

Repository
rL LLVM

Event Timeline

jlebar updated this revision to Diff 84374.Jan 13 2017, 2:00 PM
jlebar retitled this revision from to [NVPTX] Let there be One True Way to set NVVMReflect params..
jlebar updated this object.
jlebar added a reviewer: tra.
jlebar added a subscriber: llvm-commits.
tra accepted this revision.Jan 13 2017, 3:29 PM
tra edited edge metadata.
tra added inline comments.
llvm/test/CodeGen/NVPTX/nvvm-reflect.ll
4–6 ↗(On Diff #84374)

Nit. Could we construct whole input in one file instead of piping two separate files through?

cp %s %t
echo '!0...' >> %t
opt %t... | FileCheck

This would make it a bit more convenient to manually rerun these commands if necessary.

This revision is now accepted and ready to land.Jan 13 2017, 3:29 PM

Argh, this is actually wrong. I forgot that we already have a ftz function attribute. We should use that and completely nix the module attr.

I think the right thing is:

  • Use D28538 to set the ftz attr on libdevice functions.
  • Add attr-merging logic for the ftz attr so it behaves like the fastmath function attr. (I believe the merging logic is, if the callee lacks fastmath, the caller has fastmath stripped from it. But whatever the logic is, we should just do the same thing, I think.)
  • Change nvvm-reflect to look at the function-level ftz attr rather than the module ftz attr.
  • Stop setting the module-level ftz attr in clang.

...but, this is still a step in the right direction, I think we should do it and then change the pass again to look at the function metadata. Metadata merging on this attr is a little scary, so I want to take our time.

jlebar updated this revision to Diff 84494.Jan 15 2017, 8:58 AM
jlebar marked an inline comment as done.
jlebar edited edge metadata.

Use better idiom for constructing test file, as suggested by tra.

This revision was automatically updated to reflect the committed changes.