llvm dev - Dec 2015 - Objects of MemoryLocation class are created for ‘llvm.memset.*‘ intrinsics

2015-12-11 23:44 GMT+05:00 Hal Finkel <hfinkel at
anl.gov>:
> Hi Roman,
>
> The MemoryLocation objects are involved in findAliasSetForUnknownInst, but
none are created there for the memset intrinsic. MemoryLocation objects are only
involved for the regular memory accesses being compared to the unknown
instruction. See AliasSet::aliasesUnknownInst in
lib/Analysis/AliasSetTracker.cpp.
>
> We do know how to create MemoryLocation objects for memsets, see
MemoryLocation::getForDest, but we don't do so in the AliasSetTracker.
We're taking advantage here of the fast that AA's getModRefInfo
understands (or should understand) enough about memset to accurately answer the
queries.
Hi Hal,

Thank you for the explanation!


-- 
                                    Cheers, Roman Gareev.

2015-12-12 12:36 GMT+05:00 Roman Gareev <gareevroman at
gmail.com>:
> 2015-12-11 23:44 GMT+05:00 Hal Finkel <hfinkel at anl.gov>:
>> Hi Roman,
>>
>> The MemoryLocation objects are involved in findAliasSetForUnknownInst,
but none are created there for the memset intrinsic. MemoryLocation objects are
only involved for the regular memory accesses being compared to the unknown
instruction. See AliasSet::aliasesUnknownInst in
lib/Analysis/AliasSetTracker.cpp.
>>
>> We do know how to create MemoryLocation objects for memsets, see
MemoryLocation::getForDest, but we don't do so in the AliasSetTracker.
We're taking advantage here of the fast that AA's getModRefInfo
understands (or should understand) enough about memset to accurately answer the
queries.
>
> Hi Hal,
>
> Thank you for the explanation!
Hi Hal,

I'm sorry for disturbing you. Could you please advise me how to skip
checks, which are performed in AliasSet::aliasesUnknownInst, of
unknown instructions from different alias sets of an alias set tracker
that is a parameter of ‘AliasSetTracker::add(const AliasSetTracker
&AST)’?

If this wasn’t available at the moment and you could review me, I
would try to implement it. A temporary patch can be found attached. It
passes the LLVM regression tests and helps to reduce the runtime of
'opt -basicaa -licm out.opt.ll’ from 130ms to 67ms and the runtime of
'opt -basicaa -licm out.opt2.ll’ from 117ms to 62ms (out.opt.ll and
out.opt2.ll can be found on the following link
https://llvm.org/bugs/show_bug.cgi?id=23077).

P.S.: Thank you again for the explanation. I think that, in the case
of the Bug 23077, getModRefInfo works correctly. If we modify, for
example, out.opt.ll and change the third parameter of ‘llvm.memset.*’
intrinsics from i64 256 to, for example, i64 512, there is aliasing.
Furthermore, I’ve manually created objects of the MemoryLocation
class, which are corresponds to ‘llvm.memset.*’ intrinsics, and
checked whether they alias each other. The result corresponds to a
result of getModRefInfo.

-- 
                                    Cheers, Roman Gareev.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Mark-unknown-instructions-from-the-AST-parameter.patch
Type: application/octet-stream
Size: 1816 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151214/6b46ffa3/attachment.obj>

2015-12-14 19:17 GMT+05:00 Roman Gareev <gareevroman at
gmail.com>:
> 2015-12-12 12:36 GMT+05:00 Roman Gareev <gareevroman at gmail.com>:
>> 2015-12-11 23:44 GMT+05:00 Hal Finkel <hfinkel at anl.gov>:
>>> Hi Roman,
>>>
>>> The MemoryLocation objects are involved in
findAliasSetForUnknownInst, but none are created there for the memset intrinsic.
MemoryLocation objects are only involved for the regular memory accesses being
compared to the unknown instruction. See AliasSet::aliasesUnknownInst in
lib/Analysis/AliasSetTracker.cpp.
>>>
>>> We do know how to create MemoryLocation objects for memsets, see
MemoryLocation::getForDest, but we don't do so in the AliasSetTracker.
We're taking advantage here of the fast that AA's getModRefInfo
understands (or should understand) enough about memset to accurately answer the
queries.
>>
>> Hi Hal,
>>
>> Thank you for the explanation!
>
> Hi Hal,
>
> I'm sorry for disturbing you. Could you please advise me how to skip
> checks, which are performed in AliasSet::aliasesUnknownInst, of
> unknown instructions from different alias sets of an alias set tracker
> that is a parameter of ‘AliasSetTracker::add(const AliasSetTracker
> &AST)’?
>
> If this wasn’t available at the moment and you could review me, I
> would try to implement it. A temporary patch can be found attached. It
> passes the LLVM regression tests and helps to reduce the runtime of
> 'opt -basicaa -licm out.opt.ll’ from 130ms to 67ms and the runtime of
> 'opt -basicaa -licm out.opt2.ll’ from 117ms to 62ms (out.opt.ll and
> out.opt2.ll can be found on the following link
> https://llvm.org/bugs/show_bug.cgi?id=23077).
>
> P.S.: Thank you again for the explanation. I think that, in the case
> of the Bug 23077, getModRefInfo works correctly. If we modify, for
> example, out.opt.ll and change the third parameter of ‘llvm.memset.*’
> intrinsics from i64 256 to, for example, i64 512, there is aliasing.
> Furthermore, I’ve manually created objects of the MemoryLocation
> class, which are corresponds to ‘llvm.memset.*’ intrinsics, and
> checked whether they alias each other. The result corresponds to a
> result of getModRefInfo.
Hi Hal,

I'm sorry for disturbing you again. Could you please advise whom to
contact regarding a review of this patch?

-- 
                                    Cheers, Roman Gareev.