Re: [PATCH] s390: Add assembler-level include guard to insn-common-asm.h

[Andreas Grapentin]

On Nov 04 26, Steffen Eiden wrote:
> On Tue, Nov 03, 2026 at 05:39:19PM +0100, Alexander Gordeev wrote:
> > On Mon, Nov 02, 2026 at 05:51:03PM +0100, Steffen Eiden wrote:
> > 
> > Hi Steffen,
> > 
> > > The CPP include guard of insn-common-asm.h is ineffective when the file
> > > is read via a gas-level .include from inline asm. The assembler ignores
> > > all '#' lines, so every .include redefines GR_NUM/VX_NUM and fails with
> > > "Macro was already defined" if a translation unit emits more than one
> > > such include. This is not the case right now but may lead to future
> > > build issues.
> > > 
> > > Add a .ifndef guard with an L-prefixed (local, non-emitted) symbol so
> > > the file can be include multiple times per assembly unit.
> > > 
> > > Fixes: 495bc70aa3225 ("s390/fpu: Move GR_NUM / VX_NUM macros to separate header file")
> > 
> > Is there a real problem this patch fixes?
> 
> Yes. The second batch of arm-on-s390 patches use the GR_NUM macro as
> well. I'll send an update for this series soon. And it happens that the
> same compilation unit that uses the GR_NUM macro for creating a new
> instruction format (not supported by binutils) also uses some FPU stuff.
> So the asm marco will be defined twice. That's how I found this issue in
> the first place.

I just tested this and can confirm the problem. the fpu and sae headers
both asm(.include...) insn-common-asm.h which bypasses the cpp include
guards and breaks GAS because the macros become doubly-defined.

> > Do you happen to know why some other archs use .purgem in similar cases
> > (aka though appears ugly, should not we use .purgem too)?
> > 
> 
> .purgem can be used to undefine a macro for when you want to use it once.
> That's how the other arch use it. Define a macro, use it once, purge it.
> All in the same file.
> 
> We have a different use case here. We defined a reusable macro in a
> separate header. So if we want to use .purgem instead the users of the
> macro would need to purge it after they used it. This seems to britle
> for me as purging it can be forgotten and the issue can manifest way
> later in a complete separate patch.

also, .purgem itself errors when the macro is not already defined, so
won't help us here anyway.

However, if the include guards in insn-common-asm.h are too ugly, we
could also guard the asm(.include...) itself:

+#ifndef __ASM_S390_INSN_COMMON_ASM_H
 asm(".include \"asm/insn-common-asm.h\"\n");
+#endif

The same should also work if we wrap the asm(.include...) in another
header.

Best,
-A