On Wed, 30 Nov 2022, Zopolis0 via Gcc-patches wrote: > > * Each patch should have its own explanation of what it is doing and why, > > in the message body (not in an attachment). Just the commit summary line > > and ChangeLog entries aren't enough, we need the actual substantive commit > > message explaining the patch. > > The thing is, most of the patches do not need an explanation. Patches > 1-13 are just re-adding code,
Then state that in the message body (with a reference to the commit that removed the code). Just because code was removed in a given form doesn't mean it should be added back in that form. For example, patch 13, "Re-add flag_evaluation_order, reorder_operands_p, and add reorder bool argument to tree_swap_operands_p", seems suspicious. That sort of global state affecting IR semantics is best avoided; rather, the Java gimplification support should deal with ensuring the correct ordering for operations in the GIMPLE generated. Note that C++ flag_strong_eval_order (for C++17 evaluation order requirements) is specific to the front end; it doesn't require anything in expr.cc or fold-const.cc or other language-independent files. So you should do something similar for Java rather than adding back global language-independent state for this. Patches 1 and 2 don't seem to have reached the mailing list. > 20-43 and 47 are just applying treewide > changes that Java missed out on, So say for each one exactly which commit it's applying the changes for. > > How has the series been validated? > > I'm not exactly sure what you mean by this. What target triplets did you run the GCC testsuite on (before and after the changes, with no regressions), with what results for the Java-specific tests? > I plan to > replace Classpath with the OpenJDK, and double down on the machine > code aspect of GCJ, dropping bytecode and interpreted support. This sort of thing is key information to include in the summary message for any future versions of the patch series. -- Joseph S. Myers jos...@codesourcery.com
