This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Dubious dse.c code


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]