This is the mail archive of the gcc-patches@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]

Re: PATCH: fold BIT_AND_EXPR's with ADDR_EXPR operands


On 9/19/07, Ollie Wild <aaw@google.com> wrote:
> On 9/18/07, Jakub Jelinek <jakub@redhat.com> wrote:
> >
> > The tests shouldn't cast pointers to (unsigned), but to uintptr_t
> > or at least size_t, otherwise it is -Wpointer-to-int-cast material
> > on LP64 architectures.
>
> I've attached an updated patch.  After some consideration, I've opted
> to use __SIZE_TYPE__, which is consistent with several other tests
> (e.g. array-10.c) and avoids problems with missing stdint.h headers.

+      /* If arg0 is derived from the address of an object or function, we may
+        be able to fold this expression using the object or function's
+        alignment.  */
+      if (POINTER_TYPE_P (TREE_TYPE (arg0)) &&
+         TREE_CODE (arg1) == INTEGER_CST && TREE_INT_CST_HIGH (arg1) == 0)
+       {

coding style says the && goes to the next line.  Instead of
TREE_CODE (arg1) == INTEGER_CST && TREE_INT_CST_HIGH (arg1) == 0 use
host_integer_p (arg1, 1).

The description of get_pointer_modulus_and_residue isn't exactly clear
to me.  It looks
like it should compute the biggest power-of-two N and M so that p % N == M?  But
this doesn't have a single solution - so what is it exactly computing?
 Please clarify
that in the functions description.

+         expr = get_inner_reference (expr, &bitsize, &bitpos, &offset,
+                                     &mode, &unsignedp, &volatilep, false);
+         *residue = bitpos / BITS_PER_UNIT;
+         if (offset)
+           {
+             if (TREE_CODE (offset) == INTEGER_CST)
+               *residue += TREE_INT_CST_LOW (offset);
+             else
+               expr = NULL;

an INTEGER_CST offset is always accounted for in bitpos,  no need to repeat that
here, just bail out.  As for the function I would make it return the
modulus, that would
make the flow easier.

+      if (expr && DECL_P (expr))
+       *modulus = DECL_ALIGN_UNIT (expr);
+      else

expr can't be non-NULL here.  I don't see how you protect against a
residue bigger
than modulus?  But I guess it doesn't matter, still the names are
confusing then.

I would still like you to re-investigate using get_pointer_alignment.
Basically you
would split out a helper function to get the pointer expression with
its ADDR_EXPR
stripped off and do

   base = get_inner_reference (expr, ...);
   base_align = get_pointer_alignment_1 (base, -1);

where base_align yields your modulus and from the get_inner_reference result
you compute the residue.  And yes, the result from get_pointer_alignment has
to be useful for folding as strict alignment targets rely on it for
correct code.

Thanks,
Richard.


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