Previously we relied on the .s.o rule in share/mk/bsd.suffixes.mk to tell make that linux_support.o is built from linux_support.s, even though we do not use the .s.o rule to assemble it.
Details
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
also specify linux_support.s explicitly - ${IMPSRC} is not defined for explicit rules
| sys/modules/linux64/Makefile | ||
|---|---|---|
| 103 | I suppose ${.IMPSRC} was defined via the .s.o rule in bsd.suffixes.mk even though this rule here is not a suffix-transformation rule.
| |
| sys/modules/linux64/Makefile | ||
|---|---|---|
| 103 | I'll defer to @sjg for comment on the canonical way to handle this. I believe what's in this review now is workable, but perhaps it could be ${ALLSRC:[1]}? Or perhaps we should in fact have a .s.o suffix rule with no commands, and leave linux/Makefile and linux64/Makefile as is? | |
| sys/modules/linux64/Makefile | ||
|---|---|---|
| 103 | Or yet another option, we could rename the handful of .s files affected by this review to .S. | |
| sys/modules/linux64/Makefile | ||
|---|---|---|
| 103 | or to .asm? like linux_locore.asm | |
Why not have a .s.o rule that uses Clang like we do for .S.o? I think this is conflating "we don't want to use AS directly" with "we don't want non-preprocessed assembly files".
| sys/modules/linux64/Makefile | ||
|---|---|---|
| 103 | Ah, I think that would work too. .s.o:
${AS} ${AFLAGS} -o ${.TARGET} ${.IMPSRC}
${CTFCONVERT_CMD}
.S.o:
${CC:N${CCACHE_BIN}} ${CFLAGS} ${ACFLAGS} -c ${.IMPSRC} -o ${.TARGET}
${CTFCONVERT_CMD}
.asm.o:
${CC:N${CCACHE_BIN}} -x assembler-with-cpp ${CFLAGS} ${ACFLAGS} -c ${.IMPSRC} \
-o ${.TARGET}
${CTFCONVERT_CMD}so .asm and .S are mostly equivalent except that .asm invokes the compiler with -x assembler-with-cpp for the implicit rule. | |
| sys/modules/linux64/Makefile | ||
|---|---|---|
| 103 | Yes, because -x assembler-with-cpp is automatically used for .S files | |
Sorry a bit late to the party.
First off; If you add -DWITH_META_MODE to your build, you can look at linux_support.o.meta to see exactly what was done.
If you kldload filemon you can even confirm all the files used.
As man page says .IMPSRC is only guaranteed in suffix ruels.
| sys/modules/linux/Makefile | ||
|---|---|---|
| 137 | This ends up being very fragile. ${.ALLSRC:M*.s} would be far simpler | |
I expect .asm is handled the same as .S for the benefit of case insensitive file systems (Apple)
Why not have a .s.o rule that uses Clang like we do for .S.o?
Ah, just the .S.o rule but with -x assembler instead of -x assembler-with-cpp? Yeah, that makes sense.
I needed to change to :M*.s:u because linux_support.s ended up appearing twice - presumably once from the .s.o rule and once from the explicit mention in the two Makefiles. Is this expected Make behaviour?
| sys/modules/linux64/Makefile | ||
|---|---|---|
| 103 | oops lost -o here, have restored it locally | |