Thu Mar 15 14:18:00 GMT 2007
> On Wed, 14 Mar 2007, Jan Hubicka wrote:
> > > On the course of implementing this I hit some bugs in the expansion
> > > code of these builtins and the i386 backend, which are also fixed by
> > > this patch.
> > This would be probably worth backporting separately to release
> > branch(es).
> Hmm, actually I have a problem with this part now, in that I get ICEs now
> in some circumstances, not in the i386 backend, but due to my other fixes
> in builtins.c, namely these (and the similar) changes:
> @@ -5717,8 +5717,10 @@ expand_builtin_sync_operation (enum mach
> mem = get_builtin_sync_mem (CALL_EXPR_ARG (exp, 0), mode);
> val = expand_expr (CALL_EXPR_ARG (exp, 1), NULL, mode, EXPAND_NORMAL);
> - /* If VAL is promoted to a wider mode, convert it back to MODE. */
> - val = convert_to_mode (mode, val, 1);
> + /* If VAL is promoted to a wider mode, convert it back to MODE.
> + Not for CONST_INTs though. */
> + if (GET_MODE (val) != VOIDmode)
> + val = convert_to_mode (mode, val, 1);
> if (ignore)
> return expand_sync_operation (mem, val, code);
> Without this change, this call will be miscompiled on 32bit hosts:
> __sync_fetch_and_add_8 (ptr, -(unsigned long long)1);
> Because the -1LL will be 0xffffffffffffffff in the tree expression. Now
> the expand_expr call above will turn that tree constant into a RTL
> constant, namely (const_int -1 [0xffffffff]), that is correct as far as I
> can see, as RTL constants are implicitely sign extended.
> Now, without my change the convert_to_mode() call will turn that RTL
> constant into the wrong 64bit unsigned variant of this (mode is DImode,
> and due to the unsigned flag being set for the convert_to_mode call),
> namely (const_int [0xffffffff]   ), i.e. it's zero extended.
> So, my thought was to not call convert_to_mode for CONST_INTs at all,
> which seems what other code sometimes is doing. This works fine for the
> DImode case, but I'm now seeing e.g. QImode failures :-(
Hmm, is thre reason why convert_modes doesn't work?
(also gen_int_mode will get you appropriate constants)
More information about the Gcc-patches