On Tue, Aug 8, 2023 at 5:53 PM Jason Merrill <ja...@redhat.com> wrote: > > On 7/31/23 22:22, Lewis Hyatt via Gcc-patches wrote: > > `#pragma GCC target' is not currently handled in preprocess-only mode (e.g., > > when running gcc -E or gcc -save-temps). As noted in the PR, this means that > > if the target pragma defines any macros, those macros are not effective in > > preprocess-only mode. Similarly, such macros are not effective when > > compiling with C++ (even when compiling without -save-temps), because C++ > > does not process the pragma until after all tokens have been obtained from > > libcpp, at which point it is too late for macro expansion to take place. > > > > Since r13-1544 and r14-2893, there is a general mechanism to handle pragmas > > under these conditions as well, so resolve the PR by using the new "early > > pragma" support. > > > > toplev.cc required some changes because the target-specific handlers for > > `#pragma GCC target' may call target_reinit(), and toplev.cc was not > > expecting > > that function to be called in preprocess-only mode. > > > > I added some additional testcases from the PR for x86. The other targets > > that support `#pragma GCC target' (aarch64, arm, nios2, powerpc, s390) > > already had tests verifying that the pragma sets macros as expected; here I > > have added -save-temps to some of them, to test that it now works in > > preprocess-only mode as well. > > > > gcc/c-family/ChangeLog: > > > > PR preprocessor/87299 > > * c-pragma.cc (init_pragma): Register `#pragma GCC target' and > > related pragmas in preprocess-only mode, and enable early handling. > > (c_reset_target_pragmas): New function refactoring code from... > > (handle_pragma_reset_options): ...here. > > * c-pragma.h (c_reset_target_pragmas): Declare. > > > > gcc/cp/ChangeLog: > > > > PR preprocessor/87299 > > * parser.cc (cp_lexer_new_main): Call c_reset_target_pragmas () > > after preprocessing is complete, before starting compilation. > > > > gcc/ChangeLog: > > > > PR preprocessor/87299 > > * toplev.cc (no_backend): New static global. > > (finalize): Remove argument no_backend, which is now a > > static global. > > (process_options): Likewise. > > (do_compile): Likewise. > > (target_reinit): Don't do anything in preprocess-only mode. > > (toplev::main): Adapt to no_backend change. > > (toplev::finalize): Likewise. > > > > gcc/testsuite/ChangeLog: > > > > PR preprocessor/87299 > > * c-c++-common/pragma-target-1.c: New test. > > * c-c++-common/pragma-target-2.c: New test. > > * g++.target/i386/pr87299-1.C: New test. > > * g++.target/i386/pr87299-2.C: New test. > > * gcc.target/i386/pr87299-1.c: New test. > > * gcc.target/i386/pr87299-2.c: New test. > > * gcc.target/s390/target-attribute/tattr-2.c: Add -save-temps to the > > options, to test preprocess-only mode as well. > > * gcc.target/aarch64/pragma_cpp_predefs_1.c: Likewise. > > * gcc.target/arm/pragma_arch_attribute.c: Likewise. > > * gcc.target/nios2/custom-fp-2.c: Likewise. > > * gcc.target/powerpc/float128-3.c: Likewise. > > --- > > > > Notes: > > Hello- > > > > This patch fixes the PR by enabling early pragma handling for `#pragma > > GCC > > target' and related pragmas such as `#pragma GCC push_options'. I did > > not > > need to touch any target-specific code, however I did need to make a > > change > > to toplev.cc, affecting all targets, to make it safe to call > > target_reinit() > > in preprocess-only mode. (Otherwise, it would be necessary to modify > > the > > implementation of target pragmas in every target, to avoid this code > > path.) > > That was the only complication I ran into. > > > > Regarding testing, I did: (thanks to GCC compile farm for the non-x86 > > targets) > > > > bootstrap + regtest all languages - x86_64-pc-linux-gnu > > bootstrap + regtest c/c++ - powerpc64le-unknown-linux-gnu, > > aarch64-unknown-linux-gnu > > > > The following backends also implement this pragma so ought to be > > tested: > > arm > > nios2 > > s390 > > > > I am not able to test those directly. I did add coverage to their > > testsuites > > (basically, adding -save-temps to any existing test, causes it to test > > the > > pragma in preprocess-only mode.) Then, I verified on x86_64 with a > > cross > > compiler, that the modified testcases fail before the patch and pass > > afterwards. nios2 is an exception, it does not set any libcpp macros > > when > > handling the pragma, so there is nothing to test, but I did verify that > > processing the pragma in preprocess-only mode does not cause any > > problems. > > The cross compilers tested were targets arm-unknown-linux-gnueabi, > > nios2-unknown-linux, and s390-ibm-linux. > > > > Please let me know if it looks OK? Thanks! > > The c-family and C++ changes look good. David, any comments on the > toplev changes? > > Jason >
Thanks for the review! To be clear, I should continue to wait for additional feedback on the toplev.cc change, correct? I have also implemented Joseph's suggestion, to add new testcases rather than modifying existing ones. That's a pretty minor change just to the testsuite part so I did not resend the whole patch because of it, but I can if needed. -Lewis
