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

[Steffen Eiden <seiden@linux.ibm.com>]

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")
> 
> I have a problem with Fixes here:
> - as you noted above nothing fails right now
> - the same behaviour was there before 495bc70aa3225 commit
>

I have no problem with removing it. You are right this does not fix a
problem right now but prevents future issues. Let's remove it.

> 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 sent this as a separate patch in the hope that this fix gets into 7.4

> 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.

	Steffen

> > Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> > ---
> >  arch/s390/include/asm/insn-common-asm.h | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/s390/include/asm/insn-common-asm.h b/arch/s390/include/asm/insn-common-asm.h
> > index fd9b3cacb7c5..060116719846 100644
> > --- a/arch/s390/include/asm/insn-common-asm.h
> > +++ b/arch/s390/include/asm/insn-common-asm.h
> > @@ -9,6 +9,10 @@
> >  
> >  #ifdef __ASSEMBLER__
> >  
> > +/* Prevent duplicated GAS macro definitions. CPP guards have no effect for GAS.*/
> > +.ifndef .Lasm_s390_insn_common_asm_h
> > +.Lasm_s390_insn_common_asm_h = 1
> > +
> >  /*
> >   * GR_NUM - Retrieve general-purpose register number
> >   *
> > @@ -49,5 +53,7 @@
> >  	.endif
> >  .endm
> >  
> > +.endif	/* .Lasm_s390_insn_common_asm_h */
> > +
> >  #endif	/* __ASSEMBLER__ */
> >  #endif	/* __ASM_S390_INSN_COMMON_ASM_H */
> 
> Thanks!