[PATCH] D61812: [AMDGPU] Fixed handling of imemdiate i1 literals

Mon May 13 08:07:54 PDT 2019

rampitec marked an inline comment as done.
rampitec added a comment.

In D61812#1499954 <https://reviews.llvm.org/D61812#1499954>, @nhaehnle wrote:

> Why does this return false? A 1-bit immediate is either 0 or -1, both of which can be represented as inline constants everywhere.

Technically yes, but the query is about VOP literal, while i1 ends up in a SOP normally.

================
Comment at: lib/Target/AMDGPU/SIInstrInfo.cpp:2526
   switch (Imm.getBitWidth()) {
+  case 1: // This likely will be a condition code mask.
+    return false;
----------------
arsenm wrote:
> arsenm wrote:
> > rampitec wrote:
> > > arsenm wrote:
> > > > I don’t understand where this is coming from. There should be no 1-bit immediates anywhere?
> > > Combiner sometimes produces "xor x, true", and even "add x, true". This is not the first time we hit it, we have even implemented lowering.
> > I would expect this to be extended to the wavesize during selection
> I guess that wouldn't provide much benefit, but this should always be true?
Yes, these are extended, but we have the situation where predicate is checked before the extension.

While it technically can fit VOP literal I do not believe that would be a good idea. The bool operand shall go SOP instruction, so I preffered to return false. At the very least we will cancel unsuitable pattern matching earlier this way.

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

https://reviews.llvm.org/D61812