After successfully commuting an instruction to be compatible with the
current VGPR MSB mode, update CurrentMode with the commuted
instruction's mode requirements. This locks in the mode bits the
commuted instruction relies on, preventing later instructions from
piggybacking and corrupting those bits.
Without this fix, a subsequent instruction needing a different mode
could piggyback onto the preceding s_set_vgpr_msb and change mode bits
that the commuted instruction depends on. For example, a nullopt src1
position (treated as 0) could be overwritten to a different value,
causing incorrect register encoding for the commuted instruction.
The fix still allows compatible piggybacking - instructions that only
add new mode bits without changing existing ones can still piggyback.
This test uses opt to run instcombin and then pipes that into llc which
has its output piped into FileCheck. Before this patch, the test also
piped in the source file into llc as well, which caused issues with a
downstream test executor that executes the lines in bash. However, these
extra pipes don't make sense anyways, so remove them.
This patch makes it so that renumbering indices when inserting
instructions into the SlotIndexes analysis renumbers the entire list if
the list is otherwise densely packed. This fixes a case we saw on
AArch64 with a lot of spills where every single spill instruction
insertion required a renumbering of most of the instructions in a large
function, making the operation approximately quadratic.
This is not NFC as heuristics depend on the SlotIndex numbers, although
this should mostly be a wash as LRs should be extended ~equally.
BF16 source operands use F32 inline constant values, so set OP_SEL to
select the high half of the constant, since BF16 encoding matches the
high 16 bits of F32 encoding. This behaviour is different from F16
source operands which use F16 constant values in the low 16 bits.
Fixes: #183337
Two changes:
1. int_amdgcn_cvt_pk_fp/bf8_f32 takes a i32 as a tied input, and this
tied input is passed to cvt_pk_f8_fp32 as a tied vdst_in. In true16 mode
the vdst_in is in vgpr16. Since this i32 could be in sreg32, need to
copy it to vgpr32 first before extract_subreg.
2. remove SRCMODS.DST_OP_SEL from cvt_pk_f8_fp32 pattern in true16 mode.
The opsel is not needed since the hi/lo16 will be clobbered by the
following register rewrite/allocation pass and this const opsel causes
conflictions
All DPP instructions in AMDGPUGenInstrInfo.inc are verified to now be
marked as convergent. This is necessary to prevent DPP instructions from
being incorrectly sunk by machine-sink.
Signed-off-by: John Lu <John.Lu@amd.com>
This is one of the string attributes that takes a boolean
value for no reason. There is no point in ever writing this
with an explicit false. Stop adding the noise and reporting
an unnecessary change.
On GFX1250, V_NOPs inserted for WMMA coexecution hazards are placed at
the use-site. When the hazard-consuming instruction is inside a loop and
the WMMA is outside, these NOPs execute every iteration even though the
hazard only needs to be covered once.
This patch hoists the V_NOPs to the loop preheader, reducing executions
from N iterations to 1.
```
Example (assuming a hazard requiring K V_NOPs):
Before:
bb.0 (preheader): WMMA writes vgpr0
bb.1 (loop): V_NOP xK, VALU reads vgpr0, branch bb.1
-> K NOPs executed per iteration
After:
bb.0 (preheader): WMMA writes vgpr0, V_NOP xK
bb.1 (loop): VALU reads vgpr0, branch bb.1
-> K NOPs executed once
```
For nested loops, V_NOPs are hoisted to the outermost preheader where no
WMMA hazard exists within the loop.
Hoisting is restricted to strict preheaders (not any single predecessor)
to avoid introducing V_NOPs on unrelated control flow paths.
The optimization is controlled by `-amdgpu-wmma-vnop-hoisting` (default:
on).
Fixes: SWDEV-573407
There are three calls to bumpCycle in bumpNode. Prior to the first call,
we calculate NextCycle as the next cycle in which all of a given
instruction's required hardware resources (as defined by the SchedModel)
are available. Any gap between this calculated NextCycle and CurrCycle
measures stalls that must occur before we can schedule the given
instruction.
The second and third call handle adjustments that occur during or after
issuing of the instruction (e.g. if the number of microops exceeds the
issue width).
According to the documentation of HazardRec->EmitInstruction, we should
call this method when an instruction is emitted: "This callback is
invoked when an instruction is emitted, to advance the hazard state."
In the context of bumpNode, this implies that it should be called after
we bumpCycle for stalls that must occur before issue of the
instructions, but before those that occur during or after. This PR moves
the placement to do that.
In practice, this affects schedulers that use both the SchedModel and
HazardRec. Suppose we have instructions A, B and C, and partial schedule
AB. Also, suppose instruction A exclusively holds ProcResource X for 2
cycles, and B uses ProcResource X, and there is a HazardRec hazard
between B and C which requires 1 cycle stall.
Currently, we call HazardRec->EmitInstruction on B before we call
HazardRec->AdvanceCycle for the stall between A->B. Then, when deciding
whether to schedule C, HazardRec sees that a cycle has already occurred
after B, so we do not need to stall.
After this change, we HazardRec->EmitInstruction on B after we call
HazardRec->AdvanceCycle for the stall between A->B. So, HazardRec
accurately places the stall cycle between A and B. Then, when deciding
whether to schedule C, HazardRec accurately sees that no cycles have
occurred after B, so we do need to stall for 1 cycle.
Previously, power-of-2 div/rem operations wider than
MaxLegalDivRemBitWidth were excluded from IR expansion and left for
backend peephole optimizations. Some backends can fail to process such
instructions in case we switch off DAGCombiner.
Now ExpandIRInsts expands them into shift/mask sequences:
- udiv X, 2^C -> lshr X, C
- urem X, 2^C -> and X, (2^C - 1)
- sdiv X, 2^C -> bias adjustment + ashr X, C
- srem X, 2^C -> X - (((X + Bias) >> C) << C)
Special cases handled:
- Division/remainder by 1 or -1 (identity, negation, or zero)
- Exact division (sdiv exact skips bias, produces ashr exact)
- Negative power-of-2 divisors (result is negated)
- INT_MIN divisor (correct via countr_zero on bit pattern)
Proofs: https://alive2.llvm.org/ce/z/Y-iWm-
Assisted-by: Cursor // Claude Opus 4.6
I asked AI to port the device libs reference implementation.
It mostly worked, though it got the compares wrong and also
missed a fold that happened in compiler. With that fixed I get
identical DAG output, and almost the same globalisel output (differing
by an inverted compare and select). Also adjusted some stylistic
choices.
Fixes#182677
The `BFE_I32` case in `ComputeNumSignBitsForTargetNode` was not masking
the width operand with `& 0x1f`, unlike other BFE operations in the same
file. Since the hardware instruction only uses the low 5 bits of the
width field, values >= 32 passed via `@llvm.amdgcn.sbfe.i32` caused
unsigned integer underflow in the calculation:
unsigned SignBits = 32 - Width->getZExtValue() + 1;
When width > 33, this underflows, producing incorrect SignBits values.
When width == 33, SignBits becomes 0, violating the expected return
range of [1, BitWidth]. This led to assertion failures and
miscompilation where subsequent BFE narrowing operations were
incorrectly eliminated.
This patch:
- Masks the width value with `& 0x1f` to match hardware behavior
- Handles width == 0 (after masking) by returning 32 sign bits
- Adds regression tests for width values >= 32
Test memcpy lowering with complex constant length. Length is given by:
`i64 add (i64 sub (i64 16, i64 ptrtoint (ptr addrspacecast (ptr
addrspace(4) null to ptr) to i64)), i64 13)`
Thus, loop guard should not be needed.
---------
Signed-off-by: John Lu <John.Lu@amd.com>
Truncate 64-bit integral `setcc`s to their upper 32-bit operands if
enough information is known about their lower 32-bit operands, subsuming
the special cases handled in #177662.
Alive2 verification for analogous IR transformations:
[xdATxK](https://alive2.llvm.org/ce/z/xdATxK)
On the callee side, `LowerFormalArguments` marks SGPR0-3 as allocated in
`CCState` before running the CC analysis. On the caller side,
`LowerCall` (and GlobalISel's `lowerCall`/`lowerTailCall`) added the
scratch resource to `RegsToPass` without marking it in `CCState`. This
caused `CC_AMDGPU_Func` to treat SGPR0-3 as available on the caller
side, assigning user inreg args there, while the callee skipped them
without marking it in `CCState`. This caused `CC_AMDGPU_Func` to treat
SGPR0-3 as available on the caller side, assigning user inreg args
there, while the callee skipped them.
Add a test demonstrating a bug where the caller and callee disagree on
which SGPRs hold user inreg arguments when there are enough to reach the
SGPR0-3 range.
On the callee side, `LowerFormalArguments` marks SGPR0-3 as allocated in
`CCState` before the CC analysis runs. On the caller side, `LowerCall`
adds the scratch resource to `RegsToPass` without marking SGPR0-3 in
`CCState`. This causes `CC_AMDGPU_Func` to assign user inreg args to
SGPR0-3 on the caller side (they appear free) while the callee skips
them.
In the test, the caller writes arg 0 (value 42) to s0, but the callee
reads arg 0 from s16.
When we have a BITCAST and the source type is a vector with smaller
elements compared to the destination type, then we need to demand all
the source elements that make up the demanded elts for the result when
doing recursive calls to SimplifyDemandedBits,
SimplifyDemandedVectorElts and SimplifyMultipleUseDemandedBits. Problem
is that those simplifications are allowed to turn non-demanded elements
of a vector into POISON, so unless we demand all source elements that
make up the result there is a risk that the result would be more
poisonous (even for demanded elts) after the simplification.
The patch fixes some bugs in SimplifyMultipleUseDemandedBits and
SimplifyDemandedBits for situations when we did not consider the problem
described above. Now we make sure that we also demand vector elements
that "must not be turned into poison" even if those elements correspond
to bits that does not need to be defined according to the DemandedBits
mask.
Fixes#138513
`AMDGPUArgumentUsageInfo` provided a per-function map that
`lowerFormalArguments` would write each function's implicit argument
register layout into, and `passSpecialInputs` would read back when
lowering calls to look up the callee's layout. This per-function map is
redundant for all non-entry callees, which already use the same
`FixedABIFunctionInfo` register layout.
GlobalISel already used `FixedABIFunctionInfo` unconditionally. This
change makes SelectionDAG do the same.
The intrinsic has five arguments for the tensor descriptor (D#), while the fifth one is reserved for future targets, and it will be silently ignored in codegen for gfx1250.
For tensor up to 2D, only the first two D# groups are meaningful and the rest should be zero-initialized.
This patch adds target features:
- `+dpp-wavefront-shifts`, for DPP `wave_shl/rol/shr/ror`
- `+dpp-row-bcast`, for DPP `row_bcast15/31`
These DPP controls are not available in gfx10+, so these target features
enable `AMDGPURemoveIncompatibleFunctions` to remove functions that rely
on these controls when compiling for newer GPUs.
All PERMLANE instructions in AMDGPUGenInstrInfo.inc were verified to now
be marked as convergent. This is necessary to prevent PERMLANE
instructions from being incorrectly sunk by machine-sink.
---------
Signed-off-by: John Lu <John.Lu@amd.com>
Regbanklegalize rules for INTRIN_IMAGE loads and stores.
Because of very large number of different type signatures, rule specifies
only function for lowering (waterfall lowering of RsrcIdx operand if needed)
and this function also applies register banks.
Fixes problem with setting/resetting register classes in the MFMA
rewrite sched stage. The code assumed that the dest and OpC would be the
same class. This is not true if one uses subregs. This fixes issue
#177696.
On GFX9, the instruction sequencer fetches 32 bytes at a time. When an
8-byte instruction at a loop header straddles a 32-byte fetch window
boundary, the sequencer must perform two fetches after a backward
branch, incurring a delay. On GFX950, this causes additional performance
issues.
This patch adds 32-byte alignment (.p2align 5, , 4) for loop headers on
GFX950 when the first real instruction is 8 bytes. At most one s_nop (4
bytes, 1 quad-cycle before the loop) is used for padding. If more than 4
bytes of padding were needed, the 8-byte instruction would not straddle
a 32-byte boundary anyway, so alignment is skipped.
Note: the alignment decision is made during block-placement, before
si-insert-waitcnts. In loops where a 4-byte S_WAITCNT is later inserted
as the first instruction, the alignment becomes redundant but mostly
harmless (at most one extra s_nop per affected loop).
Assisted-by: Claude (Anthropic)
If a float-typed call site is marked with afn, replace the 4
flavors of pow with a faster variant.
This transforms pow, powr, pown, and rootn to __pow_fast,
__powr_fast, __pown_fast, and __rootn_fast if available. Also
attempts to handle all of the same basic folds on the new fast
variants that were already performed with the base forms. This
maintains optimizations with OpenCL when the device libs unsafe
math control library is deleted. This maintains the status quo
of how libcalls work, and only handles 4 new entry points. This
only helps with the elimination of the control library, and not
general libcall emission problems.
This makes no practical difference for HIP, which is the status
quo for libcall optimizations. AMDGPULibCalls recognizes the OpenCL
mangled names. e.g., OpenCL float "pow" is really _Z3powff but the
HIP provided function "powf" is really named _ZL4powfff, and std::pow
with float is _ZL3powff. The pass still runs for HIP, so by accident
if you used the OpenCL mangled function names, this would trigger.
Since the functions cannot yet be relied on from the library,
introduce a temporary module flag check. I'm not planning on emitting
it anywhere and it's a poor substitute for versioning the target.
Use `G_ANYEXT` instead of `G_ZEXT` when widening the source of
`G_CTLZ_ZERO_UNDEF`. The extended upper bits are immediately shifted out
by the subsequent left-shift, so zero-extending is unnecessarily
constraining.
Before:
```
%wide = G_ZEXT %src
%shifted = G_SHL %wide, sizeDiff
%result = G_CTLZ_ZERO_UNDEF %shifted
```
After:
```
%wide = G_ANYEXT %src
%shifted = G_SHL %wide, sizeDiff
%result = G_CTLZ_ZERO_UNDEF %shifted
```
