[PATCH] D141830: [NFC] Use new version of SelectionDAG::getLoad in NVPTX LowerFormalArguments

[Artem Belevich via Phabricator via llvm-commits]

tra added a comment.

In D141830#4064547 <https://reviews.llvm.org/D141830#4064547>, @gchatelet wrote:

> AFAIR there is a risk for a `bool` value to be `true` but hold a value that is not `1`.

I do not think so. Integer promotion rules guarantee that 'true' converts to 1.

  > §4.7/4 from the C++ 11 or 14 Standard, §7.8/4 from the C++ 17 Standard, §7.3.9/2 from the 20 Standard says (Integral Conversion)
  > If the source type is bool, the value false is converted to zero and the value true is converted to one.



> Also it felt suspicious to pass a `bool` in lieu of an `unsigned` so I just wanted to be absolutely sure it was not a mistake.

The last revision of the patch just explicitly does what the DAG.getLoad() oveload we've picked before did under the hood. We've intentionally provided an overload which acceps unsigned value for alignment. I do not see much of a point Creating `Align()` manually. For what it's worth your first patch variant was more readable.
Your diff still makes no functional changes.

Please do provide a test case illustrating that there's a problem that needs fixing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141830