Am 02.06.26 um 10:07 PM schrieb Harald Anlauf:
Hi Henri!
Am 02.06.26 um 1:15 PM schrieb Henri Menke:
Hi Harald,
Thanks for the quick feedback. Please find the patch for v2 attached.
Let me briefly address the requested changes point-by-point.
- Renamed FORCEINLINE to ALWAYS_INLINE and added a plain INLINE
attribute. INLINE only sets DECL_DECLARED_INLINE_P, while
ALWAYS_INLINE additionally sets DECL_DISREGARD_INLINE_LIMITS.
- The new EXT_ATTR_INLINE and EXT_ATTR_ALWAYS_INLINE values are now
appended at the end of the enum, before EXT_ATTR_LAST, so the existing
bit positions in the module-file bitmask are preserved. I also added a
source comment before the enum noting that new attributes must be
appended for this reason.
- The mutual-exclusion check now covers both INLINE and ALWAYS_INLINE
against NOINLINE.
While implementing these changes, I have also noticed a difference from
the C always_inline attribute. Because ALWAYS_INLINE sets
DECL_DISREGARD_INLINE_LIMITS directly rather than attaching an
always_inline attribute, it does not force inlining at -O0. For now I
have left it at that, because attaching an attribute to the declaration
would make this patch a bit more complicated. If you would prefer the
exact C semantics I am happy to attach the real attribute instead.
No, your current solution is really fine with me.
One note on the testsuite. The ALWAYS_INLINE test simply checks that
the procedure is inlined at -O. The plain INLINE test needs more care,
because a small procedure is inlined at -O2 regardless of the
attribute. That test therefore switches off automatic inlining with -
fno-inline-small-functions -fno-inline-functions -fno-inline-functions-
called-once and then verifies that the INLINE attribute alone re-
enables inlining for that one procedure. I added a comment in the test
explaining this, but I worry about the robustness of this check,
especially on other platforms that I cannot test.
This is also fine.
There is only one thing I noted while playing with your new testcase
inline_2.f90: depending on the ordering of the lines
!GCC$ ATTRIBUTES inline :: bar
!GCC$ ATTRIBUTES noinline :: bar
one gets a warning that either looks correct or confusing.
The way you've chosen it for the testcase I get:
13 | !GCC$ ATTRIBUTES noinline :: bar ! { dg-warning "INLINE.
at .1. is incompatible with .NOINLINE." }
| 1
Warning: Attribute »INLINE« at (1) is incompatible with »NOINLINE« for
»bar« and will be ignored
This is because the order in which the attributes are processed
is not taken into account by your new checks.
Anyway, I think this is rather a cosmetic issue and not a real problem.
Besides, there is a minor whitespace issue your can check yourself:
% ./contrib/check_GNU_style.sh 0001-fortran-add-INLINE-and-
ALWAYS_INLINE-attributes.patch
Blocks of 8 spaces should be replaced with tabs.
83:+ { "inline", EXT_ATTR_INLINE, NULL },
84:+ { "always_inline",EXT_ATTR_ALWAYS_INLINE,NULL },
It is hard to see without this hint, but after "NULL" there
shall be a tab. I can fix this myself.
Bootstrapped and regtested on x86_64-pc-linux-gnu with --enable-
languages=c,c++,fortran, no regressions.
If nobody else speaks up, particularly w.r.t. the warnings mentioned
above, I'll commit for you tomorrow.
Thanks for the patch!
Harald
No further comments, so pushed as r17-1274-ga3de5985947287 .
Thanks,
Harald
