On Aug 19, 2013, at 1:21 PM, Jeff Law <l...@redhat.com> wrote:
> On 07/30/2013 09:24 PM, DJ Delorie wrote:
>> [nickc added for comments about the bits he wrote]
>>
>>> ... define these as
>>>
>>> (define_predicate "msp_general_operand"
>>> (match_code "mem,reg,subreg,const_int,const,symbol_ref,label_ref"
>>> {
>>> int save_volatile_ok = volatile_ok;
>>> volatile_ok = 1;
>>> int ret = general_operand (op, mode);
>>> volatile_ok = save_volatile_ok;
>>> return ret;
>>> })
>>>
>>> That said, why do they exist?
>>
>> Because gcc refuses to optimize operations with volatile memory
>> references, even when the target has opcodes that follow the volatile
>> memory rules. "set bit in memory" for example. I've had to do
>> something like this for every port I've written, for the same reason,
>> despite arguing against gcc's pedantry about volatile memory accesses.
> I'd say it's not as simple as you make it out to be. You can't blindly
> combine operations on volatile memory. ie, the programmer may have written
> them as separate statements and if they're volatile you should leave them
> alone. With this change a series of statements involving volatiles might be
> combined into a single insn. That's not good.
>
> So while you will get better optimization in the presence of volatile, you'll
> also eventually get a mis-optimization of a volatile reference. And debugging
> it will be an absolute nightmare because whether or not it causes a visible
> problem will be dependent on the state of some hardware widget.
>
> I'll note that *NO* ports in the official GCC sources do this and that's
> because it's wrong. Unfortunately, I missed this when looking at the port or
> I would have called it out earlier. What you may have done for ports that
> are still in private trees is not pertinent.
First, some background for those that are new around here (you're not!):
http://gcc.gnu.org/ml/gcc-bugs/1999-12n/msg00762.html
http://gcc.gnu.org/ml/gcc/2005-11/msg00990.html
http://gcc.gnu.org/ml/gcc/2000-01/msg00400.html
http://gcc.gnu.org/ml/gcc-bugs/2000-06/msg00390.html
I disagree in part with you. The compiler is perfectly free to optimize
volatile in many different ways. That we don't, is a bug that can be fixed,
but that no one has yet bothered to do much about, other than sneaking around
in the corners. I, like you, don't favor the sneaking around in the corners.
I think the right approach is for people to flip on the optimization once the
port is clean wrt volatile and the the back end bugs that arise from it that
the port maintainer deems are reasonably under control (a port only decision).
Long term, we should define exactly what the ports must do (and not do), so
that we can default on the optimizations. All new ports should just be
required to be correct wrt these requirements.
An example of what can be optimized:
volatile int a;
int main() {
int b;
b = a;
++b;
a = b;
}
Some machines have suitable instructions to do effectively an atomic add &a, 1.
This doesn't violate the spirit of the language standard or the sensibilities
of people that write volatile code. Of course, such code now would seem
positivity quaint given <atomic> (aka lib atomic) type things.
I do understand that there are conditions that need to be checked for the code
to be flawless, and that absent those checks, it won't be. I just don't agree
that dodging the issue for another 13 years is the best path forward.
So, re-cap, we aren't talking about doing anything blindly. Separate
statements can be combined. Combining them is good. One can combine things
incorrectly, and we are not arguing that case. Only the things that are
correct, see above for one example.
[ glancing at trunk ]
Oh, cute, create_fixed_operand. :-) Someone has attacked the issue in a
sideways fashion. Nice start, though, doesn't get at the heart of the problem.
More constructive if for those that know of conditions that need to be checked,
that are not, would instead add the checks, then, trivially, we can just turn
it on and be done with it. Slight less useful, but still productive, would be
list give examples of codes that do the wrong thing. For me personally, I know
of no bugs. I think the port should experiment with just flipping the big
switch, and because I didn't provide a link or a patch last time, let me
include it now:
diff --git a/gcc/recog.c b/gcc/recog.c
index 0defd7d..a93eacd 100644
--- a/gcc/recog.c
+++ b/gcc/recog.c
@@ -1012,7 +1012,7 @@ general_operand (rtx op, enum machine_mode mode)
{
rtx y = XEXP (op, 0);
- if (! volatile_ok && MEM_VOLATILE_P (op))
+ if (! TARGET_VOLATILE_CORRECT && ! volatile_ok && MEM_VOLATILE_P (op))
return 0;
/* Use the mem's mode, since it will be reloaded thus. */
So that others know exactly what I'm thinking about. Targets that fix their
port to be clean with respect to volatile (you must not read or write in a
manner other than is directly allowed if volatile is set) can turn it one and
go on with life.
So, does anyone have a concrete example of what will not work on the msp430
port, if this is done?
