Volatile operations and PRE
Hello,
I have discovered that volatile expresions can cause the tree-ssa
pre pass to loop forever in "compute_antic". The problem seems to be
that the expresion is assigned a different value number at each
iteration, hence the fixed point required to exit the loop is never reached.
This can be fixed with the attached patch, which modifies
"can_value_number_operation" to return false for volatile expresions. I
think this makes sense, because you cannot value number volatile
expresions (in the same sense that you cannot value number non pure or
const function calls).
I cannot easily provide a testcase because this problem appears only
with a gcc frontend that I am writting. With this fix, volatile acesses
work correctly (without it they work correctly only if this pass is
disabled).
Do you think this patch is correct?
Thanks,
Ricardo.
Index: gcc/tree-ssa-pre.c
===
--- gcc/tree-ssa-pre.c (revision 557)
+++ gcc/tree-ssa-pre.c (working copy)
@@ -2133,12 +2133,13 @@
static bool
can_value_number_operation (tree op)
{
- return UNARY_CLASS_P (op)
-|| BINARY_CLASS_P (op)
-|| COMPARISON_CLASS_P (op)
-|| REFERENCE_CLASS_P (op)
-|| (TREE_CODE (op) == CALL_EXPR
- && can_value_number_call (op));
+ return (UNARY_CLASS_P (op)
+ || BINARY_CLASS_P (op)
+ || COMPARISON_CLASS_P (op)
+ || REFERENCE_CLASS_P (op)
+ || (TREE_CODE (op) == CALL_EXPR
+ && can_value_number_call (op)))
+&& !TREE_THIS_VOLATILE (op);
}
Re: Volatile operations and PRE
Thank you for your answer. I give some more information below:
Daniel Berlin wrote:
On 11/6/06, Ricardo FERNANDEZ PASCUAL <[EMAIL PROTECTED]> wrote:
Hello,
I have discovered that volatile expresions can cause the tree-ssa
pre pass to loop forever in "compute_antic". The problem seems to be
that the expresion is assigned a different value number at each
iteration, hence the fixed point required to exit the loop is never
reached.
This should not be possible, it would imply that you have SSA names
marked as volatile, or a statement whose operands are marked volatile
but ann->has_volatile_ops is false.
Actually, I have a MODIFY_EXPR with a volatile operand whose
ann->has_volatile is not being set.
Any statement which contains volatile operands should have
ann->has_volatile_ops set.
That is your real bug.
Ok, the tree that is giving me problems is the following one:
unit size
align 32 symtab 0 alias set -1 precision 32 min max
pointer_to_this >
side-effects visited
arg 0
visited var def_stmt
version 3>
arg 1
side-effects volatile
arg 0
static SI file volatile.exe line 1 size unit
size
align 32 chain >
arg 1
SI file volatile.exe line 1 size unit size
align 32 offset_align 128
offset
bit offset context >>
volatile.exe:1>
Notice that the arg 1 of the MODIFY_EXPR is a COMPONENT_REF which is
marked as volatile. Notice also that the arg 1 of the COMPONENT_REF is
not marked as such, because that field is not volatile itself and there
are other accesses to it which are not volatile. This is because in my
source language individual load or stores can be marked as volatile, not
the variables.
So, I think the real question is: are COMPONENT_REF nodes allowed to be
marked as volatile by themselves? I think they should, and actually it
seems to work (the generated code looks correct).
If it is allowed, the attached patch would solve my problem too. Would
it be correct this time?
Thanks for your help,
Ricardo.
Index: ../gcc/tree-ssa-operands.c
===
--- ../gcc/tree-ssa-operands.c (revision 557)
+++ ../gcc/tree-ssa-operands.c (working copy)
@@ -1960,7 +1960,7 @@
if (code == COMPONENT_REF)
{
- if (s_ann && TREE_THIS_VOLATILE (TREE_OPERAND (expr, 1)))
+ if (s_ann && (TREE_THIS_VOLATILE (TREE_OPERAND (expr, 1)) || TREE_THIS_VOLATILE (expr)))
s_ann->has_volatile_ops = true;
get_expr_operands (stmt, &TREE_OPERAND (expr, 2), opf_none);
}
Re: Volatile operations and PRE
Andrew Haley wrote:
Ricardo FERNANDEZ PASCUAL writes:
> So, I think the real question is: are COMPONENT_REF nodes allowed
> to be marked as volatile by themselves? I think they should, and
> actually it seems to work (the generated code looks correct).
volatile is a type qualifier. The type of a COMPONENT_REF is the type
of the operand to which it refers. If you want to change the
effective type of a reference, you should generate a suitable
CONVERT_EXPR. Like this:
tree exp_type = TREE_TYPE (exp);
tree v_type
= build_qualified_type (exp_type,
TYPE_QUALS (exp_type) | TYPE_QUAL_VOLATILE);
tree addr = build_fold_addr_expr (exp);
v_type = build_pointer_type (v_type);
addr = fold_convert (v_type, addr);
exp = build_fold_indirect_ref (addr);
Thank you. I have tried this and it works for stores, but not for loads
(for loads it behaves as a non volatile load).
I have done some experiments to try to understand what is happening, and
I am a bit confused by the bahavior of GCC. Consider the following C
function:
static struct { int w; } s;
void wait (void) {
int t;
loop:
t = *((volatile int *) &s.w);
if (t > 0) goto loop;
}
The code generated by "cc1 -O3" on x86 is:
wait:
movls, %eax
pushl %ebp
movl%esp, %ebp
testl %eax, %eax
jg .L6
popl%ebp
ret
.L3:
.L6:
jmp .L6
Which does not seem to respect the semantics of volatile. Is this the
expected behavior or is this a bug?
FWIW, the folowing function:
void wait2 (void) {
int t;
volatile int *p = &s.w;
loop:
t = *p;
if (t > 0) goto loop;
}
generates the following code, which seems correct to me:
wait2:
pushl %ebp
movl%esp, %ebp
.p2align 4,,7
.L9:
movls, %eax
testl %eax, %eax
jg .L9
popl%ebp
ret
Thanks in advance for any help.
Best regards,
Ricardo.
Re: Volatile operations and PRE
Paolo Bonzini wrote: At a wild guess, maybe strip_useless_type_conversions() is doing something Bad. Almost there. It looks like strip_useless_type_conversions is not used, and then something Bad happens. Thank you Andrew and Paolo for your quick answers. I have made a report for this bug (#29753). I will test your patch tomorrow. Best regards, Ricardo.
Re: Volatile operations and PRE
Paolo Bonzini wrote: The following patch fixes it, but it's completely untested. 2006-11-07 Paolo Bonzini <[EMAIL PROTECTED]> * gimplify.c (fold_indirect_ref_rhs): Use STRIP_USELESS_TYPE_CONVERSION rather than STRIP_NOPS. Index: ../../gcc/gimplify.c === --- ../../gcc/gimplify.c(revision 118481) +++ ../../gcc/gimplify.c(working copy) @@ -3207,7 +3207,7 @@ fold_indirect_ref_rhs (tree t) tree sub = t; tree subtype; - STRIP_NOPS (sub); + STRIP_USELESS_TYPE_CONVERSION (sub); subtype = TREE_TYPE (sub); if (!POINTER_TYPE_P (subtype)) return NULL_TREE; If anybody could regtest it, this is a regression from 3.3, likely to be present in all 4.x branches. I have tested it and ran the test suite and it does not seem to introduce problems in gcc or g++. I am using the gcc-4.3-20061104 snapshot and the results from http://gcc.gnu.org/ml/gcc-testresults/2006-11/msg00151.html. Best regards, Ricardo.
Explicit field layout
Hello, I am writing a new GCC front end. One of the features provided by my language (CIL) is explicit field layout and size for records. I don't know if any other languaje supported by GCC provides this feature. If that's the case, please point me to it so that I can see how to do it. For the case of explicit record size, what I am doing currently is modifying the TYPE_SIZE() of the declaration after the call to layout_type. It seems to work, but I am not sure if this is the correct way to do this. As for setting the explicit field offset, I haven't tried to do it yet. Would it be enough to set the DECL_FIELD_OFFSET and DECL_FIELD_BIT_OFFSET fields of each field declaration manually? When should it be done (after calling layout_type or before)? Any pointers would be greatly appreciated. Thank you in advance, Ricardo Fernández Pascual.
Re: Explicit field layout
Duncan Sands wrote: I am writing a new GCC front end. One of the features provided by my language (CIL) is explicit field layout and size for records. I don't know if any other languaje supported by GCC provides this feature. If that's the case, please point me to it so that I can see how to do it. Ada provides this. See for example section 13.5 of the reference manual (http://www.adaic.com/standards/index.html). Thanks, this is precisely what I need. Ada seems to provide the same features (actually, a superset of what CIL provides). Best regards, Ricardo.
Re: Explicit field layout
Bernd Jendrissek wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Fri, Sep 22, 2006 at 02:58:26PM +0200, Ricardo FERNANDEZ PASCUAL wrote: I am writing a new GCC front end. One of the features provided by my language (CIL) is explicit field layout and size for records. Any pointers would be greatly appreciated. Are you sure you want to do that, instead of using, say, rpcgen? I am sorry, but I fail to see the relation of this with rpcgen (which as far I know is a code generator for the RPC protocol). Am I looking at the wrong rpcgen? Best regards, Ricardo.
