[llvm] r372285 - GlobalISel: Don't materialize immarg arguments to intrinsics

[Hans Wennborg via llvm-commits]

This broke the Chromium build, causing a "error in backend: Cannot
select: t362: v4i32 = X86ISD::VSHLI" (or variations thereof) error.

See https://bugs.chromium.org/p/chromium/issues/detail?id=1005750#c1
for a reproducer.

I've reverted this and the follow-ups in r372314 in the meantime.


On Thu, Sep 19, 2019 at 3:31 AM Matt Arsenault via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
> Author: arsenm
> Date: Wed Sep 18 18:33:14 2019
> New Revision: 372285
>
> URL: http://llvm.org/viewvc/llvm-project?rev=372285&view=rev
> Log:
> GlobalISel: Don't materialize immarg arguments to intrinsics
>
> Encode them directly as an imm argument to G_INTRINSIC*.
>
> Since now intrinsics can now define what parameters are required to be
> immediates, avoid using registers for them. Intrinsics could
> potentially want a constant that isn't a legal register type. Also,
> since G_CONSTANT is subject to CSE and legalization, transforms could
> potentially obscure the value (and create extra work for the
> selector). The register bank of a G_CONSTANT is also meaningful, so
> this could throw off future folding and legalization logic for AMDGPU.
>
> This will be much more convenient to work with than needing to call
> getConstantVRegVal and checking if it may have failed for every
> constant intrinsic parameter. AMDGPU has quite a lot of intrinsics wth
> immarg operands, many of which need inspection during lowering. Having
> to find the value in a register is going to add a lot of boilerplate
> and waste compile time.
>
> SelectionDAG has always provided TargetConstant for constants which
> should not be legalized or materialized in a register. The distinction
> between Constant and TargetConstant was somewhat fuzzy, and there was
> no automatic way to force usage of TargetConstant for certain
> intrinsic parameters. They were both ultimately ConstantSDNode, and it
> was inconsistently used. It was quite easy to mis-select an
> instruction requiring an immediate. For SelectionDAG, start emitting
> TargetConstant for these arguments, and using timm to match them.
>
> Most of the work here is to cleanup target handling of constants. Some
> targets process intrinsics through intermediate custom nodes, which
> need to preserve TargetConstant usage to match the intrinsic
> expectation. Pattern inputs now need to distinguish whether a constant
> is merely compatible with an operand or whether it is mandatory.
>
> The GlobalISelEmitter needs to treat timm as a special case of a leaf
> node, simlar to MachineBasicBlock operands. This should also enable
> handling of patterns for some G_* instructions with immediates, like
> G_FENCE or G_EXTRACT.
>
> This does include a workaround for a crash in GlobalISelEmitter when
> ARM tries to uses "imm" in an output with a "timm" pattern source.