Re: [google] Use R_ARM_GOT_PREL to simplify global address loading from GOT (issue4433079)

[Carrot Wei <carrot at google dot com>]

On Thu, Apr 28, 2011 at 10:08 PM,  <dnovillo@google.com> wrote:
> I only have some stylistic comments for this patch. ?The new pass looks
> OK to me, but I do not know this area well enough to do a good review.
>
> In your ChangeLog entries, please remove the directory prefix from the
> file names.
>
done.

>
> http://codereview.appspot.com/4433079/diff/1/gcc/hooks.c
> File gcc/hooks.c (right):
>
> http://codereview.appspot.com/4433079/diff/1/gcc/hooks.c#newcode287
> gcc/hooks.c:287: return NULL;
> +hook_rtx_void_null (void)
> +{
> + ?return NULL;
>
> s/NULL/NULL_RTX/
>
done.

> http://codereview.appspot.com/4433079/diff/1/gcc/simplify-got.c
> File gcc/simplify-got.c (right):
>
> http://codereview.appspot.com/4433079/diff/1/gcc/simplify-got.c#newcode83
> gcc/simplify-got.c:83: return (optimize > 0) &&
> targetm.got_access.get_pic_reg ();
> +{
> + ?return (optimize > 0) && targetm.got_access.get_pic_reg ();
>
> s/(optimize > 0)/optimize/
>
done.

> http://codereview.appspot.com/4433079/diff/1/gcc/simplify-got.c#newcode118
> gcc/simplify-got.c:118: if (!(set && (SET_DEST (set) == pic_reg)))
> + ? ? ? ? /* If an insn both set and use pic_reg, it is in the process of
> + ? ? ? ? ? ?constructing the value of pic_reg. We should also ignore it.
> ?*/
> + ? ? ? ? rtx set = single_set (insn);
> + ? ? ? ? if (!(set && (SET_DEST (set) == pic_reg)))
>
> Extra ( ) around SET_DEST are not needed.
>
done.

> http://codereview.appspot.com/4433079/
>

The revised patch is attached.

thanks
Carrot

Attachment: patch.diff
Description: Binary data