RFR(S) 8077504: Unsafe load can loose control dependency and cause crash

[Andrew Dinn]

On 22/04/15 17:18, Roland Westrelin wrote:
> Thanks for taking a look at this. See my answer below.
> . . .
> I tried to use a code pattern that is used elsewhere in c2:
> 
> LoadLNode* LoadLNode::make_atomic(Node* ctl, Node* mem, Node* adr, const TypePtr* adr_type, const Type* rt, MemOrd mo) {
>   bool require_atomic = true;
>   return new LoadLNode(ctl, mem, adr, adr_type, rt->is_long(), mo, require_atomic);
> }
> 
> 
> I understand does_not_depend_only_on_test is confusing. I use that
> pattern because other options didn’t feel better and I can’t say I find
> it great.
> 
> Vladimir suggested privately to set _depends_only_on_test to true in
> the constructor and then use an explicit call to a new a method
> set_depends_only_on_test() to set it to false in the rare cases where
> it’s needed. That feels better indeed. What do you think?

Yes, I think that would be better.

By the way I should have made it clear I had nothing else to add. Apart
from this detail the patch looks fine as a way to avoid this load (and
/only/ this load) being moved up.

regards,


Andrew Dinn
-----------