On Tue, Sep 20, 2011 at 1:13 PM, Tom de Vries <vr...@codesourcery.com> wrote:
> Hi Richard,
>
> I have a patch for PR43814. It introduces an option that assumes that function
> arguments of pointer type are aligned, and uses that information in
> tree-ssa-ccp. This enables the memcpy in pr43814-2.c to be inlined.
>
> I tested the patch successfully on-by-default on x86_64 and i686 (both gcc
> only
> builds).
>
> I also tested the patch on-by-default for ARM (gcc/glibc build). The patch
> generated wrong code for uselocale.c:
> ...
> glibc/locale/locale.h:
> ...
> /* This value can be passed to `uselocale' and may be returned by
> it. Passing this value to any other function has undefined behavior. */
> # define LC_GLOBAL_LOCALE ((__locale_t) -1L)
> ...
> glibc/locale/uselocale.c:
> ...
> locale_t
> __uselocale (locale_t newloc)
> {
> locale_t oldloc = _NL_CURRENT_LOCALE;
>
> if (newloc != NULL)
> {
> const locale_t locobj
> = newloc == LC_GLOBAL_LOCALE ? &_nl_global_locale : newloc;
>
> ...
> The assumption that function arguments of pointer type are aligned, allowed
> the
> test 'newloc == LC_GLOBAL_LOCALE' to evaluate to false.
> But the usage of ((__locale_t) -1L) as function argument in uselocale violates
> that assumption.
>
> Fixing the definition of LC_GLOBAL_LOCALE allowed the gcc tests to run without
> regressions for ARM.
>
> Furthermore, the patch fixes ipa-sra-2.c and ipa-sra-6.c regressions on ARM,
> discussed here:
> - http://gcc.gnu.org/ml/gcc-patches/2011-08/msg00930.html
> - http://gcc.gnu.org/ml/gcc-patches/2011-09/msg00459.html
>
> But, since glibc uses this construct currently, the option is off-by-default
> for
> now.
>
> OK for trunk?
No, I don't like to have an option to control this. And given the issue
you spotted it doesn't look like the best idea either. This area in GCC
is simply too fragile right now :/
It would be nice if we could extend IPA-CP to propagate alignment
information though.
And at some point devise a reliable way for frontends to communicate
alignment constraints the middle-end can rely on (well, yes, you could
argue that's what TYPE_ALIGN is about, and I thought that maybe we
can unconditionally rely on TYPE_ALIGN for pointer PARM_DECLs
at least - your example shows we can't).
In the end I'd probably say the patch is ok without the option (thus
turned on by default), but if LC_GLOBAL_LOCALE is part of the
glibc ABI then we clearly can't do this.
Richard.
> Thanks,
> - Tom
>
> 2011-09-20 Tom de Vries <t...@codesourcery.com>
>
> PR target/43814
> * tree-ssa-ccp.c (get_align_value): New function, factored out of
> get_value_from_alignment.
> (get_value_from_alignment): Use get_align_value.
> (get_value_for_expr): Use get_align_value to handle alignment of
> function argument pointers.
> * common.opt (faligned-pointer-argument): New option.
> * doc/invoke.texi (Optimization Options): Add
> -faligned-pointer-argument.
> (-faligned-pointer-argument): New item.
>
> * gcc/testsuite/gcc.dg/pr43814.c: New test.
> * gcc/testsuite/gcc.target/arm/pr43814-2.c: New test.
>
