[PATCH] D154225: [X86] Enable the VR512 register class when AVX512 is enabled

Fri Jun 30 09:39:45 PDT 2023

craig.topper added a comment.

In D154225#4464336 <https://reviews.llvm.org/D154225#4464336>, @n-omer wrote:

>> I think this is not the right fix. We intended to define legal types under useAVX512Regs.
>
> I agree with you that this specific fix may not be the right but at the moment when "asked" whether 512 bit types are legal the backend "says" no even though AVX512 is explicitly enabled (see `TargetLowering::getRegForInlineAsmConstraint` for concrete details), this appears to be the cause of this bug.
>
>> I think a proper fix should be setting "min-legal-vector-width"="512" when parsing the inline asembly in FE.
>
> I'm not at all familiar with Clang which happens to the front-end in this case but wouldn't the front-end have to ask the backend for this information?
> IIUC the first time inline assembly is really scrutinized is during DAG construction in `SelectionDAGBuilder::visitInlineAsm` (please correct me if I'm wrong), and that is the point at which we try to discover which registers are used in the inline assembly and which registers are clobbered by it so that they can be appropriately represented in the DAG. This is where `TargetLowering::getRegForInlineAsmConstraint` comes into play and things go wrong.

MS inline assembly is fully parsed during the frontend to detect the registers that are written. This adds the `~{zmm6}` you can see on the statement in IR.

CGStmt.cpp in the frontend updates LargestLegalVectorWidth and thus min-legal-vector-width for inline assembly inputs and outputs, but I missed clobbers.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154225