http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59477

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P2                          |P3
                 CC|                            |ebotcazou at gcc dot gnu.org,
                   |                            |jakub at gcc dot gnu.org,
                   |                            |uros at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r175063, but I guess that just made the problem no longer latent.
More reduced testcase:

struct A
{
  unsigned *a, b;
  A (unsigned x) : a (), b (x) {}
};

struct B
{
  B (int);
  B (const B &) {}
};

B bar (B, B, A);
int v;

void
foo ()
{
  B c = 0;
  bar (c, c, A (1ULL << v));
}

I don't think this is a RA bug, the problem is I think that combine changes:
 (insn 20 19 21 2 (set (reg:DI 2 cx)
         (const_int 0 [0])) pr59477.C:20 62 {*movdi_internal_rex64}
      (nil))

-(insn 21 20 22 2 (set (reg:SI 37 r8)
-        (subreg:SI (reg:DI 64) 0)) pr59477.C:20 64 {*movsi_internal}
-     (expr_list:REG_DEAD (reg:DI 64)
-        (nil)))
+(insn 21 20 22 2 (parallel [
+            (set (reg:DI 37 r8)
+                (ashift:DI (reg:DI 65)
+                    (subreg:QI (reg:SI 63 [ v ]) 0)))
+            (clobber (reg:CC 17 flags))
+        ]) pr59477.C:20 503 {*ashldi3_1}
+     (expr_list:REG_UNUSED (reg:CC 17 flags)
+        (expr_list:REG_DEAD (reg:SI 63 [ v ])
+            (expr_list:REG_DEAD (reg:DI 65)
+                (nil)))))

and thus moves a complex operation (which in this case will need %ecx register)
to a place where hard regs need to be live across that and thus makes it hard
for RA to do it's job.

Usually combiner will reject such combination on i?86/x86_64 in the fn argument
hard reg setup insns because of cant_combine_insn_p, but here r8 isn't likely
spilled.  Also, typically cx is initialized after r8, because hard register
arguments are initialized from last argument to first.  But in this case there
is one argument passed in rcx/r8 pair and within a single argument we typically
initialize the first half before the second one.

As no arguments are passed in more than two hard registers, supposedly
the rcx/r8 case is the only problematic one for function arguments, so perhaps
ix86_legitimate_combined_insn could if the output is hard register r8 look for
previous insn if it sets rcx and in that case reject the combination.

In general, e.g. for local register asm variables (e.g. as used for various
syscall inline asm wrappers etc.), I wonder if it isn't unsafe to combine
anything across instructions that set likely spilled hard registers, so perhaps
alternative would be to take that into account when creating LOG_LINKS in
create_log_links.  As clearing the whole next_use array upon seeing
  if (HARD_REGISTER_NUM_P (regno)
      && ! TEST_HARD_REG_BIT (fixed_reg_set, regno)
      && targetm.class_likely_spilled_p (REGNO_REG_CLASS (regno)))
early in the
  for (def_vec = DF_INSN_DEFS (insn); *def_vec; def_vec++)
loop might be too expensive, perhaps we'd need to add another array with a tick
count when next_use was set and increment the tick count on the above mentioned
condition, and consider uses with tick count smaller than the current one as
next_use[] == NULL.

Thoughts on this?

Reply via email to