On Mon, 4 Nov 2019, luoxhu wrote: > -finline-functions is enabled by default for O2 since r276469, update the > test cases with -fno-inline-functions. > > v2: disable inlining for the failed cases. Add two more failed cases > not listed in BZ. Tested on P8LE, P8BE and P9LE.
If inlining (or other interprocedural analysis) invalidates a test's intent (e.g. all the code gets optimized away), our normal approach is to use noinline etc. function attributes to prevent that inlining. If you're adding such options to work around an ICE, which certainly appears to be the case in the architecture-independent testcases here, you should (a) have comments in the tests saying explicitly that the options are there temporarily to work around the ICE in a bug whose number is given in the comment, and (b) a remark in the open regression bug for the ICE saying that those options have been added as a temporary workaround and that a patch fixing the ICE should remove them again. The commit message also needs to make very clear that the commit is *not* a fix for that bug and so it must *not* be closed as fixed until there is an actual fix for the ICE. So I don't think this patch is OK without having such comments in the tests to explain the issue and a carefully written commit message warning that the patch is a workaround, not a fix and the bug in question must not be closed simply because of the commit mentioning it. -- Joseph S. Myers jos...@codesourcery.com
