[PATCH] D81651: [AMDGPU][CODEGEN] Added support of new inline assembler constraints

Wed Jun 24 07:31:45 PDT 2020

arsenm added inline comments.

================
Comment at: llvm/docs/LangRef.rst:4151
+- ``J``: A 16-bit signed integer constant.
+- ``L``: A 15-bit unsigned integer constant.
 - ``A``: An integer or a floating-point inline constant.
----------------
dp wrote:
> arsenm wrote:
> > dp wrote:
> > > arsenm wrote:
> > > > This one is weird; we don't actually have any of these instructions?
> > > Sure, we do not have anything like this in AMDGPU ISA encodings and I'm not sure how useful this constraint would be. However we should probably support it for the sake of compatibility with GCC. 
> > I'm not sure we need to aim for perfect GCC compatibility. I mean why did they add all of these? Is anything using them? Should they have?
> I have no idea. Some time ago I looked at GCC mail threads but only found a commit adding these constraint. There was no discussion, no rationale behind adding specific constraints.
> 
> So as we do not aim for 100% compatibility with GCC, I’ll update this change and remove support for ‘Kf’ and ‘L’ constraints.
I would at least split the ones that we don't understand into a separate patch. I'm not entirely comfortable spreading features we don't understand

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

https://reviews.llvm.org/D81651