Redundant constants in coremark crc8 for RISCV/aarch64 (no-if-conversion)

[Vineet Gupta <vineetg@rivosinc.com>]

[-- Attachment #1: Type: text/plain, Size: 2760 bytes --]

Hi,

When analyzing coremark build for RISC-V, noticed redundant constants 
not being eliminated. While this is a recurrent issue with RV, this 
specific instance is not unique to RV as I can trigger similar output on 
aarch64 with -fno-if-conversion, hence something which could be 
addressed in common passes.

-O3 -march=rv64gc_zba_zbb_zbc_zbs

crcu8:
	xor	a3,a0,a1
	andi	a3,a3,1
	srli	a4,a0,1
	srli	a5,a1,1
	beq	a3,zero,.L2

	li	a3,-24576	# 0xFFFF_A000
	addi	a3,a3,1		# 0xFFFF_A001
	xor	a5,a5,a3
	zext.h	a5,a5

.L2:
	xor	a4,a4,a5	
	andi	a4,a4,1	
	srli	a3,a0,2	
	srli	a5,a5,1	
	beq	a4,zero,.L3	

	li	a4,-24576	# 0xFFFF_A000
	addi	a4,a4,1		# 0xFFFF_A001
	xor	a5,a5,a4	
	zext.h	a5,a5		

.L3:
	xor	a3,a3,a5	
	andi	a3,a3,1	
	srli	a4,a0,3	
	srli	a5,a5,1	
	beq	a3,zero,.L4

	li	a3,-24576	# 0xFFFF_A000
	addi	a3,a3,1		# 0xFFFF_A001
...
...

I see that with small tests cse1 is able to substitute redundant 
constant reg with equivalent old reg.

cse_insn
    make_regs_equiv()
    ..
    canon_reg()

e.g.
void foo(void)
{
    bar(2072, 2096);
}

foo:
     li	a0,4096
     addi	a1,a0,-2000
     addi	a0,a0,-2024
     tail	bar

And it seems to even work across basic blocks.

e.g.

void foo(int x)	# -O2
{
     bar1(2072);
     foo2();
     if (x)
	bar2(2096);
}

foo:
  ...
     li s1, 4096
     addi a0, s1, -2024
  ...
     addi a0, a1, -2000
     tail bar2

It seems cse redundancy elimination logic is scoped to "paths" created 
by cse_find_path() and those seems to chain only 2 basic blocks.

;; Following path with 17 sets: 2 3
;; Following path with 15 sets: 4 5
;; Following path with 15 sets: 6 7
;; Following path with 15 sets: 8 9
;; Following path with 15 sets: 10 11
;; Following path with 15 sets: 12 13
;; Following path with 15 sets: 14 15
;; Following path with 13 sets: 16 17
;; Following path with 2 sets: 18
...

While in this case the BBs containing the dups are 6, 8, ...

(note 36 35 37 6 [bb 6] NOTE_INSN_BASIC_BLOCK)
(insn 37 36 38 6 (set (reg:DI 196)
         (const_int -24576 [0xffffffffffffa000])) 
"../../coremark-crc8.c":23:17 -1
      (nil))
...
(note 54 53 55 8 [bb 8] NOTE_INSN_BASIC_BLOCK)
(insn 55 54 56 8 (set (reg:DI 207)
         (const_int -24576 [0xffffffffffffa000])) 
"../../coremark-crc8.c":23:17 -1
      (nil))

The path construction logic in cse seems non trivial for someone just 
starting to dig into that code so I'd appreciate some insight.

Also I'm wondering if cse is the right place to do this at all, or 
should it be done in gcse/PRE ?

TIA
-Vineet

P.S. With the proposed RV cond ops extension if-conversion could elide 
this specific instance of problem, however redundant constants are a 
common enough nuisance in RV codegen whcih we need to tackle, specially 
when it is typically a 2 instruction seq.

[-- Attachment #2: coremark-crc8.c --]
[-- Type: text/x-csrc, Size: 536 bytes --]

typedef unsigned short ee_u16;
typedef unsigned char ee_u8;

ee_u16
crcu8(ee_u8 data, ee_u16 crc)
{
    ee_u8 i = 0, x16 = 0, carry = 0;

    for (i = 0; i < 8; i++)
    {
        x16 = (ee_u8)((data & 1) ^ ((ee_u8)crc & 1));
        data >>= 1;

        if (x16 == 1)
        {
            crc ^= 0x4002;
            carry = 1;
        }
        else
            carry = 0;
        crc >>= 1;
        if (carry)
            crc |= 0x8000;	// set MSB bit
        else
            crc &= 0x7fff;	// clear MSB bit
    }
    return crc;
}