RFR(S): 8136473: failed: no mismatched stores, except on raw memory: StoreB StoreI

[Roland Westrelin]

That fix wasn’t sufficient so here is a new webrev:

http://cr.openjdk.java.net/~roland/8136473/webrev.02/

On platforms that don’t support unaligned accesses we still have mismatched accesses because the java code of Unsafe.putIntUnaligned() is implemented in terms of putShort() and putByte() so my test case fails on sparc. I added a new boolean to MemNode to record whether the node is “mismatched” in addition to the “unaligned” boolean.

Roland.

> On Sep 22, 2015, at 2:50 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> 
> Okay.
> 
> Vladimir
> 
> On 9/22/15 8:47 PM, Roland Westrelin wrote:
>> For the record, it seems _unaligned is a reserved keyword for MSVC so I will change the field and function names to unaligned_access:
>> 
>> diff --git a/src/share/vm/opto/memnode.hpp b/src/share/vm/opto/memnode.hpp
>> --- a/src/share/vm/opto/memnode.hpp
>> +++ b/src/share/vm/opto/memnode.hpp
>> @@ -40,7 +40,7 @@
>>  // Load or Store, possibly throwing a NULL pointer exception
>>  class MemNode : public Node {
>>  private:
>> -  bool _unaligned;
>> +  bool _unaligned_access;
>>  protected:
>>  #ifdef ASSERT
>>    const TypePtr* _adr_type;     // What kind of memory is being addressed?
>> 
>> @@ -129,8 +129,8 @@
>>    // the given memory state?  (The state may or may not be in(Memory).)
>>    Node* can_see_stored_value(Node* st, PhaseTransform* phase) const;
>> 
>> -  void set_unaligned() { _unaligned = true; }
>> -  bool is_unaligned() const { return _unaligned; }
>> +  void set_unaligned_access() { _unaligned_access = true; }
>> +  bool is_unaligned_access() const { return _unaligned_access; }
>> 
>>  #ifndef PRODUCT
>>    static void dump_adr_type(const Node* mem, const TypePtr* adr_type, outputStream *st);
>> 
>> Roland.
>> 
>>> On Sep 22, 2015, at 9:43 AM, Roland Westrelin <roland.westrelin at oracle.com> wrote:
>>> 
>>>> Roland I had a look too, the code looks good.
>>> 
>>> Thanks, Michael.
>>> 
>>> Roland.
>>> 
>>>> 
>>>> -Michael
>>>> 
>>>> -----Original Message-----
>>>> From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-bounces at openjdk.java.net] On Behalf Of Roland Westrelin
>>>> Sent: Monday, September 21, 2015 2:03 AM
>>>> To: hotspot compiler
>>>> Subject: Re: RFR(S): 8136473: failed: no mismatched stores, except on raw memory: StoreB StoreI
>>>> 
>>>> Thanks for the review, Vladimir.
>>>> 
>>>> Roland.
>>>> 
>>>>> On Sep 21, 2015, at 3:32 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>>> 
>>>>> This is good. Thank you for extending the test.
>>>>> 
>>>>> Vladimir
>>>>> 
>>>>> On 9/18/15 11:03 PM, Roland Westrelin wrote:
>>>>>>>>> Hmm, so you just relaxed the assert. You may need to fix EA too because it also checks for matching types(memory sizes).
>>>>>>>>> I remember we had problem with RAW accesses back when jsr292 was implemented. So EA gave up on RAW access and marks allocations as escaping. Also may be superword/vectorization will be affected.
>>>>>>>>> 
>>>>>>>>> In general it is very bad for C2 to have different memory on the same non-RAW memory.
>>>>>>>> 
>>>>>>>> It already happens with vectorization. Do you see another solution (rather than relaxing the assert)? Putting a MemBarCPUOrder before and after the unaligned store?
>>>>>>> 
>>>>>>> MemBarCPUOrder are definitely will help and it is simplest solution but, I think, it is too drastic.
>>>>>>> It affect control flow and other memory slices.
>>>>>>> We can try to mark such memory accesses or/and mark objects which have such accesses (if it is possible) and prevent vectorization and scalarization for them only.  We should be able execute optimization on other memory slices then.
>>>>>> 
>>>>>> Here is a new change:
>>>>>> 
>>>>>> http://cr.openjdk.java.net/~roland/8136473/webrev.01/
>>>>>> 
>>>>>> Unaligned accesses are explicitly marked as such.
>>>>>> 
>>>>>> I added a test case with a non escaping allocation and it works fine. I also added test case with vectorization and if the accesses are effectively unaligned vectorization doesn't happen. One of the test cases causes an assertion to fire when the allocation is expanded: the logic in InitializeNode::complete_stores() is confused by the unaligned store so I made sure unaligned stores are not captured by the initialization.
>>>>>> 
>>>>>> Roland.
>>>>>> 
>>>> 
>>> 
>>