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

Tue May 14 08:23:50 PDT 2019

arsenm added inline comments.

================
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:
> rampitec wrote:
> > 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.
> But SOP instructions have the same inline immediate values. This isn't a property of the immediate or instruction
OK, I see this is breaking not at the point I thought. In the bool case these still end up materialized with regular int64_t operands. This is only happening in the DAG patterns. Saying this is true should be fine, since it still works with the normal immediate folding heuristics.

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

https://reviews.llvm.org/D61812