Re: [igt-dev] [PATCH i-g-t 3/4] assembler/gen4asm.h: Make struct src_operand extern

[Lyude Paul <lyude@redhat.com>]

On Tue, 2020-03-17 at 12:56 -0400, Lyude Paul wrote:
> On Mon, 2020-03-16 at 23:08 +0000, Chris Wilson wrote:
> > Quoting Lyude (2020-03-16 16:11:31)
> > > From: Lyude Paul <lyude@redhat.com>
> > > 
> > > Another spot where we fail compilation with -fcommon, due to src_operand
> > > getting included in multiple translation units without being marked as
> > > extern:
> > > 
> > > /usr/bin/ld:
> > > assembler/1ca89e8@@intel-gen4asm@exe/meson-
> > > generated_gram.c.o:assembler/gen4asm.h:119:
> > > multiple definition of `src_operand';
> > > assembler/1ca89e8@@intel-gen4asm@exe/meson-
> > > generated_lex.c.o:assembler/gen4asm.h:119:
> > > first defined here
> > > /usr/bin/ld:
> > > assembler/1ca89e8@@intel-gen4asm@exe/main.c.o:assembler/gen4asm.h:119:
> > > multiple definition of `src_operand';
> > > assembler/1ca89e8@@intel-gen4asm@exe/meson-
> > > generated_lex.c.o:assembler/gen4asm.h:119:
> > > first defined here
> > > 
> > > So, fix this by marking struct src_operand as extern.
> > > 
> > > Signed-off-by: Lyude Paul <lyude@redhat.com>
> > > ---
> > >  assembler/gen4asm.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/assembler/gen4asm.h b/assembler/gen4asm.h
> > > index 6b957e28..7845060c 100644
> > > --- a/assembler/gen4asm.h
> > > +++ b/assembler/gen4asm.h
> > > @@ -111,7 +111,7 @@ struct regtype {
> > >   * This structure is the internal representation of source operands in
> > > the
> > >   * parser.
> > >   */
> > > -struct src_operand {
> > > +extern struct src_operand {
> > >         struct brw_reg reg;
> > >         int default_region;
> > >         uint32_t imm32; /* set if src_operand is expressing a branch
> > > offset */
> > 
> > I suspect this was a typedef. I can't see any definition for the struct
> > src_operand src_operand, and I think it can be deleted.
> 
> You sure about this? It appears to be referenced in assembler/gram.y:
> 
> assembler/gram.y:57:static struct src_operand src_null_reg =
> assembler/gram.y:77:static struct src_operand ip_src =
> assembler/gram.y:92:				struct src_operand *src,
> assembler/gram.y:95:				struct src_operand *src,
> (etc.)
> 
> Or are you suggesting we just move it into gram.y?

ah-whoops, I just realized you were talking about the fact we defined it as a
variable at all, oops. will send a respin with this changed

> > -Chris
> > 
-- 
Cheers,
	Lyude Paul (she/her)
	Associate Software Engineer at Red Hat

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev