| Message ID | 20141007121356.GA18003@sepie.suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Oct 07, 2014 at 02:13:56PM +0200, Michal Marek wrote: > This violates the principle of least surprise: > > make $file.s > as -o $file.o $file.s > > should be equivalent to > > make $file.o I know but we need to enable -g for .s targets so that we get the .loc annotation (i.e., line numbers) in asm which is very helpful. But the least surprise principle is a valid point. Maybe we should warn about it too when building .s targets...? Or, maybe I should try to find out whether there's another gcc option which adds ".loc" annotations alone... > Why not simply check both READABLE_ASM and DEBUG_INFO? Also, it's more > straightforward to print the warning in the top-level Makefile rule than > to add a conditional to the generic rule, like this: The problem here is that we're building a couple of .s targets regardless of what the make command contains, like bounds.s and such. And we want to issue the warning only when we have an .s file as an explicit target on the command line. That's why I'm doing that asm_target assignment dance and something similar to WARN_ON_ONCE... Thanks.
On 2014-10-07 14:27, Borislav Petkov wrote: > On Tue, Oct 07, 2014 at 02:13:56PM +0200, Michal Marek wrote: >> This violates the principle of least surprise: >> >> make $file.s >> as -o $file.o $file.s >> >> should be equivalent to >> >> make $file.o > > I know but we need to enable -g for .s targets so that we get the .loc > annotation (i.e., line numbers) in asm which is very helpful. > > But the least surprise principle is a valid point. Maybe we should warn > about it too when building .s targets...? > > Or, maybe I should try to find out whether there's another gcc option > which adds ".loc" annotations alone... Such option would be best of course. BTW, do you know about make $file.lst to produce an 'annotated disassembly'? >> Why not simply check both READABLE_ASM and DEBUG_INFO? Also, it's more >> straightforward to print the warning in the top-level Makefile rule than >> to add a conditional to the generic rule, like this: > > The problem here is that we're building a couple of .s targets > regardless of what the make command contains, like bounds.s and such. The toplevel Makefile rule (where your patch adds the asm_target=$@ variable) is only used for manual invocation. bounds.s and the like are handled by Makefile.build directly. Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 07, 2014 at 03:45:43PM +0200, Michal Marek wrote: > Such option would be best of course. BTW, do you know about make > $file.lst to produce an 'annotated disassembly'? Yep. Although I sometimes find it harder to parse than looking at .loc-annotated asm and the c-source. > The toplevel Makefile rule (where your patch adds the asm_target=$@ > variable) is only used for manual invocation. bounds.s and the like are > handled by Makefile.build directly. Ah ok, I'll keep that in mind next time. I was seeing the warning being issued multiple times, thus the once-tracking.
Hey Michal, On Tue, Oct 07, 2014 at 02:13:56PM +0200, Michal Marek wrote: > This violates the principle of least surprise: > > make $file.s > as -o $file.o $file.s > > should be equivalent to > > make $file.o on a second thought, I think we don't care about this. Because we build the objects in kbuild with make and not with gas directly. IOW, we never use the intermittent product file.s when building file.o but we start again from file.c. And besides, even if we did use gas, we would still need to pass KBUILD_AFLAGS to it. Hmmm.
diff --git a/Makefile b/Makefile index 106f300..2b4f62f 100644 --- a/Makefile +++ b/Makefile @@ -1505,6 +1505,9 @@ else endif %.s: %.c prepare scripts FORCE +ifeq ($(CONFIG_READABLE_ASM)$(CONFIG_DEBUG_INFO),) + $(warning Enable CONFIG_READABLE_ASM and CONFIG_DEBUG_INFO more helpful asm) +endif $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@) %.i: %.c prepare scripts FORCE $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)