[PATCH] D138661: [AMDGPU][MC] Correct handling of mandatory literals

[Dmitry Preobrazhensky via Phabricator via llvm-commits]

dp added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:3387
+  if (!AMDGPU::isSISrcOperand(Desc, OpIdx) ||
+      AMDGPU::isKImmOperand(Desc, OpIdx)) {
     return false;
----------------
Joe_Nash wrote:
> dp wrote:
> > dp wrote:
> > > Joe_Nash wrote:
> > > > This looks a bit weird to me because isSISrcOperand used to be true for KImm, until I created the mandatory literal logic. I'm not sure what the desired semantics of isSISrcOperand are. Can you try making isSISrcOperand used to be true for KImm and seeing if there is any fallout?
> > > `isSISrcOperand` does return true for `KImm` and seems it always did. I just added a limitation on `KImm` operands - they can no longer be interpreted as inline constants.
> > > 
> > > Before the mandatory literal logic was introduced, this limitation on `KImm` operands was unnecessary because only one literal was allowed for affected instructions.
> > > 
> > > Could you describe your concern in more detail?
> > Are you suggesting to see if it is feasible to make `isSISrcOperand` return false for `KImm`?
> > Are you suggesting to see if it is feasible to make `isSISrcOperand` return false for `KImm`?
> 
> Yes.
> 
> Sorry, I said the reverse of what I meant. 
I prepared a separate change on top of this patch to see how this can be done. Please take a look. I'm not sure if this would be that useful.

https://reviews.llvm.org/D139101


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

https://reviews.llvm.org/D138661