[3.1.1] x86-64 fixes

Jan Hubicka jh@suse.cz
Fri May 24 00:10:00 GMT 2002


> I'm the grader. :-)
:)
> 
> >There are about 10 places, where asynchoronous-unwind-tables is put in
> >similar context and it is documented how it should work in invoke.texi.
> >Should I add similar comment to all the places around the code?
> 
> That can't be quite true:
> 
>  bash-2.05$ find . -name '*.c' -o -name '*.h' -o -name '*.md' | \
>             xargs -n10 grep 'asynchronous_unwind' | \
>             wc -l
>  9
> 
> And some of those are just declarations. :-) :-)
> 
> Comments won't hurt.  If the logic is duplicated, it should be
> encapsulated in a macro or function so that it is not duplicated
> any more. :-)
OK, I will take a look if it can be macroized.
> 
> This change is OK with a comment that explains what's going on.  For
> the branch, we can live without the comment; on the mainline we need it.

OK, I will add the commentary.
> 
> >Number of patches are already condtionalized by TARGET_64BIT.  Is that
> >enought?  Can you take a look at these?
> >This includes the thunk code of unix.h partially as well - the function
> >is verbatim copy of the macro just with TARGET_64BIT block changed.
> 
> I can't *tell* that without staring at it hard.  For the branch, it
> would have been better to make a simpler change.  In general, doing
> cleanups at the same time as fixes is a bad idea on the branch; it's hard
> to see what's going on.  I want to see an *obviously* safe patch.
> 
> This patch is OK, now that you've explained it to me.  You stil need
> to add comments for the function parameters though, on the mainline at
> least.  And there really ought to be comments in the middle of that
> function explaining the code that we're generating.

OK, for next time I will allways prepare two versions, one with moving
the macro offline for mainline and one into macro for the branch. There
ar at least two other macros (for profiling) that needs to be fixed.
THanks!

Honza
> 
> Thanks,
> 
> --
> Mark Mitchell                   mark@codesourcery.com
> CodeSourcery, LLC               http://www.codesourcery.com



More information about the Gcc-patches mailing list