OpenHarmony/third_party_mesa3d
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

intel/fs: Fix a cmod prop bug when the source type of a mov doesn't match the dest type of scan_inst

Browse Source 
We were previously operating with the mindset "a MOV is just a compare
with zero."  As a result, we were trying to share as much code between
the MOV path and the CMP path as possible.  However, MOV instructions
can perform type conversions that affect the result of the comparison.
There was some code added to better handle this for cases like

    and(16)         g31<1>UD       g20<8,8,1>UD   g22<8,8,1>UD
    mov.nz.f0(16)   null<1>F       g31<8,8,1>D

The flaw in these changed special cases is that it allowed things like

    or(8)           dest:D  src0:D  src1:D
    mov.nz(8)       null:D  dest:F

Because both destinations were integer types, the propagation was
allowed.  The source type of the MOV and the destination type of the OR
do not match, so type conversion rules have to be accounted for.

My solution was to just split the MOV and non-MOV paths with completely
separate checks.  The "else" path in this commit is basically the old
code with the BRW_OPCODE_MOV special case removed.

The new MOV code further splits into "destination of scan_inst is float"
and "destination of scan_inst is integer" paths.  For each case I
enumerate the rules that I belive apply.  For the integer path, only the
"Z or NZ" rules are listed as only NZ is currently allowed (hence the
conditional_mod assertion in that path).  A later commit relaxes this
and adds the rule.

The new rules slightly relax one of the previous rules.  Previously the
sizes of the MOV destination and the MOV source had to be the same.  In
some cases now the sizes can be different by the following conditions:

  - Floating point to integer conversion are not allowed.

  - If the conversion is integer to floating point, the size of the
    floating point value does not matter as it will not affect the
    comparison result.

  - If the conversion is float to float, the size of the destination
    must be greater than or equal to the size of the source.

  - If the conversion is integer to integer, the size of the destination
    must be greater than or equal to the size of the source.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12045>
This commit is contained in:
Ian Romanick
2021-08-17 17:15:03 -07:00
parent 0797388dc2
commit b23432c540
2 changed files with 103 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

89
src/intel/compiler/brw_fs_cmod_propagation.cpp
View File

@@ -320,37 +320,68 @@ opt_cmod_propagation_local(const intel_device_info *devinfo, bblock_t *block)
if (inst->opcode == BRW_OPCODE_AND)
break;
/* Not safe to use inequality operators if the types are different
*/
if (scan_inst->dst.type != inst->src[0].type &&
inst->conditional_mod != BRW_CONDITIONAL_Z &&
inst->conditional_mod != BRW_CONDITIONAL_NZ)
break;
/* Comparisons operate differently for ints and floats */
if (scan_inst->dst.type != inst->dst.type) {
/* Comparison result may be altered if the bit-size changes
* since that affects range, denorms, etc
*/
if (type_sz(scan_inst->dst.type) != type_sz(inst->dst.type))
break;
/* We should propagate from a MOV to another instruction in a
* sequence like:
*
* and(16) g31<1>UD g20<8,8,1>UD g22<8,8,1>UD
* mov.nz.f0(16) null<1>F g31<8,8,1>D
*/
if (inst->opcode == BRW_OPCODE_MOV) {
if ((inst->src[0].type != BRW_REGISTER_TYPE_D &&
inst->src[0].type != BRW_REGISTER_TYPE_UD) ||
(scan_inst->dst.type != BRW_REGISTER_TYPE_D &&
scan_inst->dst.type != BRW_REGISTER_TYPE_UD)) {
if (inst->opcode == BRW_OPCODE_MOV) {
if (brw_reg_type_is_floating_point(scan_inst->dst.type)) {
/* If the destination type of scan_inst is floating-point,
* then:
*
* - The source of the MOV instruction must be the same
* type.
*
* - The destination of the MOV instruction must be float
* point with a size at least as large as the destination
* of inst. Size-reducing f2f conversions could cause
* non-zero values to become zero, etc.
*/
if (scan_inst->dst.type != inst->src[0].type)
break;
}
} else if (brw_reg_type_is_floating_point(scan_inst->dst.type) !=
brw_reg_type_is_floating_point(inst->dst.type)) {
if (!brw_reg_type_is_floating_point(inst->dst.type))
break;
if (type_sz(scan_inst->dst.type) > type_sz(inst->dst.type))
break;
} else {
/* If the destination type of scan_inst is integer, then:
*
* - The source of the MOV instruction must be integer with
* the same size.
*
* - If the conditional modifier is Z or NZ, then the
* destination type of inst must either be floating point
* (of any size) or integer with a size at least as large
* as the destination of inst.
*/
if (!brw_reg_type_is_integer(inst->src[0].type) ||
type_sz(scan_inst->dst.type) != type_sz(inst->src[0].type))
break;
assert(inst->conditional_mod == BRW_CONDITIONAL_NZ);
if (brw_reg_type_is_integer(inst->dst.type) &&
type_sz(inst->dst.type) < type_sz(scan_inst->dst.type))
break;
}
} else {
/* Not safe to use inequality operators if the types are
* different.
*/
if (scan_inst->dst.type != inst->src[0].type &&
inst->conditional_mod != BRW_CONDITIONAL_Z &&
inst->conditional_mod != BRW_CONDITIONAL_NZ)
break;
/* Comparisons operate differently for ints and floats */
if (scan_inst->dst.type != inst->dst.type) {
/* Comparison result may be altered if the bit-size changes
* since that affects range, denorms, etc
*/
if (type_sz(scan_inst->dst.type) != type_sz(inst->dst.type))
break;
if (brw_reg_type_is_floating_point(scan_inst->dst.type) !=
brw_reg_type_is_floating_point(inst->dst.type))
break;
}
}