public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
To: Bernd Schmidt <bschmidt@redhat.com>,
	"gcc-patches@gcc.gnu.org"	<gcc-patches@gcc.gnu.org>
Cc: Richard Biener <rguenther@suse.de>, Jeff Law <law@redhat.com>
Subject: Re: [PATCH] Make basic asm implicitly clobber memory, pr24414
Date: Wed, 09 Dec 2015 16:32:00 -0000	[thread overview]
Message-ID: <VI1PR07MB091197A52F2936524659E072E4E80@VI1PR07MB0911.eurprd07.prod.outlook.com> (raw)
In-Reply-To: <56684D4B.9020308@redhat.com>

Hi,

On 09.12.2015 16:48 Bernd Schmidt wrote:
> On 12/09/2015 04:09 PM, Bernd Edlinger wrote:
>
>> So would you agree on the general direction of the patch,
>> if I drop the hunk in sched-deps.c ?
>
> I'm not sure there was any consensus in that other thread, but I think 
> assuming that basic asms clobber memory and CC, can be justified. That 
> certainly seems like a safer default. Ideally though I think it would 
> be best if we could deprecate basic asms in functions, or at least 
> warn about them in -Wall.
>
>

Well no, we did not get to a consensus on the warning issue.

My personal gut feeling on that warning is a bit mixed...

If we have a -Wall-enabled warning on asm("..."), people who know next 
to nothing
about assembler will be encouraged to "fix" this warning in a part of 
the code which
they probably do not understand at all. This frightens me a bit.

Because I know they will soon find out, that adding a few colons fixes 
the warning, but
asm("...":::) is not any better IMHO.

For me, it is just very very unlikely that any piece of assembler really 
clobbers nothing
and has no inputs and no outputs at the same time, even it it looks so 
at first sight...

It is much more likely that someone forgot to fill in the clobber section.

So for me it would also be good to warn on asm("...":::) and require 
that, if they want
to fix this warning, they must at least write something in the clobber
section, like asm ("...":::"nothing"); that would be a new clobber name 
which can only
stand alone and, which can get stripped after the warning processing 
took place in the FE.

So I think a warning should warn on something that is so unusual that it 
is likely a bug.


Bernd.

      reply	other threads:[~2015-12-09 16:32 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-09  2:18 Bernd Edlinger
2015-12-09 11:06 ` Bernd Schmidt
2015-12-09 15:09   ` Bernd Edlinger
2015-12-09 15:48     ` Bernd Schmidt
2015-12-09 16:32       ` Bernd Edlinger [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=VI1PR07MB091197A52F2936524659E072E4E80@VI1PR07MB0911.eurprd07.prod.outlook.com \
    --to=bernd.edlinger@hotmail.de \
    --cc=bschmidt@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=law@redhat.com \
    --cc=rguenther@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).