[llvm-dev] Dealing with illegal operand mappings in RegBankSelect

[Quentin Colombet via llvm-dev]

> On Feb 26, 2019, at 3:30 PM, Matt Arsenault <arsenm2 at gmail.com> wrote:
> 
> 
> 
>> On Feb 21, 2019, at 12:18 AM, Quentin Colombet via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>> 
>> Hi Matt,
>> 
>>> On Feb 20, 2019, at 4:49 PM, Arsenault, Matthew via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>>> 
>>> Hi,
>>>  
>>> Some operations on AMDGPU require operands which must be in a register bank that is impossible to copy from another. The operation needs to be rewritten in a complex way to avoid the illegal copy.
>>>  
>>> Currently if I correctly report the required register banks in the operand mapping, RegBankSelect happily inserts the illegal copies, somehow concluding they are cheap (I would at least hope there’s an assert or something).
>> 
>> I would have hoped that we end up with a huge cost.
>> The assumption is that you can copy to/from any pair of register bank, then the idea is that the copy should be lowered in something that makes sense (e.g., a pair of store/load) by the target.
>> I am guessing that we could directly ask the target what pseudo-copy should be inserted instead of always adding plain copy.
>> 
>> Would that work for you?
> 
> You can’t legitimately copy from vector to scalar. It conceptually doesn’t work, and going through memory doesn’t help. The use instruction needs to be rewritten to (in the worst case) scalarize the operation for every workitem. A pseudocopy would still be some illegal operation which cannot exist which would need to be guaranteed to be removed, so I don’t think this would be any cleaner than allowing the illegal copies.
> 
> 
>> 
>>> So far I’ve worked around this by lying and reporting all of the invalid source register banks as legal. applyMapping then checks these registers and creates the new vregs and rewrites as necessary. Is this the way this is intended to work?
>> 
>> I am not sure I understand the work around.
>> Let me try to illustrate this with an example.
>> Let say we have:
>> = inst a(bank1)
>> And inst only accepts bank2.
>> Are you saying that you lie by saying that bank1 is legal in that case?
>> If so, why applyMapping would do any rewriting since this is legal.
> 
> Yes. It’s what happens here: https://reviews.llvm.org/D58511 <https://reviews.llvm.org/D58511>
> 
> For extract_vector_elt, the indirect register index type must be an SGPR. I’ve implemented the mapping as just copying the original source bank and allowing applyMapping to deal with it.

I don’t get what you mean by “applyMapping to deal with it”. Won’t applyMapping will insert copies to rewrite these, hence how is this different from what the repairing code does?
Unless, maybe, you’re talking about the target specific RegisterBankInfo::applyMapping not RegBankSelect::applyMapping.

If that’s the case what do you do in here that we could maybe generalize?

> There might be cases that aren’t correctly being rewritten, but I haven’t finished handling every case to run into them yet.
> 
>> 
>>> This sort of makes sense, since it is legal in the sense that applyMapping can deal with it. The cases that need to be rewritten can then have a very high, but not impossible cost in the case of getInstrAlternativeMappings.
>>>  
>>> Lying about the actually legal banks seems counterintuitive to me.
>> 
>> Agree, that’s not the way it’s supposed to work.
>> 
>>> Should there be something like a new RepairingPlacement type for this to just create the vregs without inserting the copy?
>> 
>> See my previous comment on what the copies are supposed to mean. I guess we could imagine a new kind of repairing placement but I am not sure what it would bring us, in the sense of lowering the (pseudo) copy differently should already do what we want.
>> 
> 
> It would avoid producing a totally illegal operation that something would need to be responsible for ensuring is deleted
> 
> -Matt

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190226/9d6d6b18/attachment-0001.html>