While looking at the Darwin (const (minus ...)) thing[*],
I came across some dubious-looking code in dse.c:

                  /* The groups are different, if the alias sets
                     conflict, clear the entire group.  We only need
                     to apply this test if the read_info is a cselib
                     read.  Anything with a constant base cannot alias
                     something else with a different constant
                     base.  */
                  if ((read_info->group_id < 0)
                      && canon_true_dependence (group->base_mem, 
                                                QImode,
                                                group->canon_base_mem,
                                                read_info->mem, rtx_varies_p))
                    {
                      if (kill)
                        bitmap_ior_into (kill, group->group_kill);
                      bitmap_and_compl_into (gen, group->group_kill);
                    }

The thing to note is that:

  - the first argument to canon_true_dependence is supposed to
    be the canonicalised MEM and
  - the third argument is supposed to be the canonicalised address

All other calls in dse.c seem to be correct.  But in this one,
we pass the uncanoncalised MEM and canonicalised MEM respectively.
Thus the first argument isn't canonicalised when we expect it to be,
and the all-important address argument has the wrong value.

Luckily, passing a MEM address usually causes canon_true_dependence
to return true, so we're generally safe.

I would fix this myself, but I don't understand why the code is
algporithmically safe.  We create base_mem as follows:

      gi->base_mem = gen_rtx_MEM (QImode, base);
      ...
      gi->canon_base_mem = canon_rtx (gi->base_mem);

I.e. the MEM is always a single byte, regardless of the real access size.
Moreover, group_info generally describes a _range_ of BASE+OFFSET MEMs,
not a single MEM.  There's no guarantee that BASE+0 is ever actually used.
So how can a true dependence check on (mem:QI BASE) be enough?

Also, this MEM has alias set 0, which seems to contradict the first
part of the comment.

For avoidance of doubt, base_mem and canon_base_mem only exist for
this canon_true_dependence call.

Richard

[*] Which is going well btw.  Hope to submit by this weekend.

Reply via email to