On Mon, Nov 27, 2023 at 02:20:25PM +0100, Rainer Orth wrote:
> Hi Jakub,
>
> >> 2023-11-23 Rainer Orth <r...@cebitec.uni-bielefeld.de>
> >>
> >> libsanitizer:
> >> PR libsanitizer/112563
> >> * configure.ac (libsanitizer_cv_as_sym_assign): Check for
> >> assembler symbol assignment support.
> >> * configure, config.h.in: Regenerate.
> >> * sanitizer_common/sanitizer_redefine_builtins.h: Include config.h.
> >> Check HAVE_AS_SYM_ASSIGN.
> >
> > Can you please
> > 1) split it into 2 patches, one touching config* which is owned by GCC (and
> > Makefiles, see later), one just
> > sanitizer_common/sanitizer_redefine_builtins.h
> > 2) avoid using config.h in, instead use AC_SUBST and add
> > @HAVE_AS_SYM_ASSIGN@
> > to Makefile.am's DEFS where needed (either expanding to nothing or
> > -DHAVE_AS_SYM_ASSIGN=1)? The reason is to minimize changes to imported
> > sources
> >
> > Once the sanitizer_common/sanitizer_redefine_builtins.h change (just
> > the && defined(HAVE_AS_SYM_ASSIGN) addition) patch is committed and pushed
> > upstream, add its commit has LOCAL_PATCHES.
>
> But will they accept a patch to check a macro never set anywhere in and
> irrelevant to LLVM? That's why I kept all in one patch, to be GCC-local.
I meant the patch would be gcc local.
But, for later we need only the changes to the imported files be in one
commit, not others, because merge.sh will not revert the GCC owned changes,
just the imported ones, and so that is what should be reapplied.
And, the preference of not using config.h is because we do it like that
for other stuff already (exactly to minimize amount of local changes).
> If we go (or at least try) this upstream route, should I wait for
> approval there and than commit both parts to GCC, keeping it in my local
> tree until then?
>
> > Note, your ChangeLog entry was pretending config.h include has been added
> > to one header, but it went to a different one instead.
>
> Drats, that's what you get for starting one way and adjusting later ;-)
Jakub
