https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91865

--- Comment #2 from Jozef Lawrynowicz <jozefl.gcc at gmail dot com> ---
A related issue can be observed if "char i" is made global instead of an
argument to func.

const int table[2] = {1, 2};

int foo;
char i;

void func(void)
{
  foo = table[i];
}

Combine combines the zero_extend and sign_extend insns into a single
(sign_extend(subreg)) insn:

Trying 6 -> 7:
    6: r27:HI=zero_extend([`i'])
    7: r28:PSI=sign_extend(r27:HI)#0
      REG_DEAD r27:HI
Failed to match this instruction:
(set (reg:PSI 28 [ i ])
    (sign_extend:PSI (subreg:HI (mem/c:QI (symbol_ref:PSI ("i")

As far as I'm aware, this is a "discouraged" use of subreg, but also means we
miss out on the best match.

Note that the subreg in the combined instruction is unrelated to the subreg
that is the rvalue of insn 7. The subreg in insn 7 is from the RTL pattern for
extendhipsi2.
To avoid any confusion I've attached a one line patch
(msp430-extendhispsi2.diff) which removes the the subreg from this RTL insn
pattern, yet the the subreg in the insn combine searches for remains. Here is
the output from combine with the patch (note the the lack of subreg in insn 7):

Trying 6 -> 7:                                                                  
    6: r27:HI=zero_extend([`i'])                                                
    7: r28:PSI=sign_extend(r27:HI)                                              
      REG_DEAD r27:HI                                                           
Successfully matched this instruction:                                          
(set (reg:PSI 28 [ i ])                                                         
    (sign_extend:PSI (subreg:HI (mem/c:QI (symbol_ref:PSI ("i")
allowing combination of insns 6 and 7                                           
original costs 16 + 8 = 24                                                      
replacement cost 20                                                             
deferring deletion of insn with uid = 6.                                        
modifying insn i3     7: r28:PSI=sign_extend([`i']#0)                           
deferring rescan insn with uid = 7.

Even though we matched, it is undesirable since we matched the costly
"extendhipsi2".
We want to match movqipsi2, which would happen if:
(sign_extend:PSI (zero_extend:HI (mem:QI))) -> (zero_extend:PSI (mem:QI))

P.S. I believe the subreg in some of the msp430 RTL patterns is because of
historical issues with reload and/or optimization tweaks. The attached patch is
just to clarify the behaviour I mentioned and not necessarily beneficial
overall.

Reply via email to