[LLVMdev] Bug in SelectionDAG visitTargetIntrinsic

[Duncan Sands]

Hi Evan,

On 07/11/12 00:42, Evan Cheng wrote:
>
>   void SelectionDAGBuilder::visitTargetIntrinsic(const CallInst &I,
> -                                               unsigned Intrinsic) {
> -  bool HasChain = !I.doesNotAccessMemory();
> -  bool OnlyLoad = HasChain && I.onlyReadsMemory();
> +                                             unsigned Intrinsic) {
> +  // Info is set by getTgtMemInstrinsic
> +  TargetLowering::IntrinsicInfo Info;
> +  bool IsTgtIntrinsic = TLI.getTgtMemIntrinsic(Info, I, Intrinsic);
> +  bool HasChain = Info.readMem || Info.writeMem;
> +  bool OnlyLoad = HasChain && Info.readMem;
>
> This doesn't seem right. If a call is marked ReadNone, it doesn't seem legal to
> select it to an intrinsic that read / write memory. By definition a "ReadNone"
> function cannot touch memory, no?

you can have a readnone call to a function that writes memory, it's just that
you somehow know that this particular call doesn't write memory.  For example
consider a call to
   int @my_func(int param, bool write_mem);
which only writes memory if write_mem is 'true'.  Then it would be correct
to mark calls to my_func "readonly" when the write_mem parameter is 'false',
even though the my_func function itself could't have the readonly parameter,
since it can write memory sometimes, i.e. when write_mem is 'true'.

Presumably Micah has an intrinsic that works this way: it is capable of writing
memory, but in some special identifiable circumstances it doesn't.

Ciao, Duncan.

>
> Evan
>
> On Nov 6, 2012, at 1:45 PM, "Villmow, Micah" <Micah.Villmow at amd.com
> <mailto:Micah.Villmow at amd.com>> wrote:
>
>> *From:*Villmow, Micah
>> *Sent:*Tuesday, November 06, 2012 1:37 PM
>> *To:*'llvm-dev at cs.uiuc.edu <mailto:llvm-dev at cs.uiuc.edu>'
>> *Cc:*Guo, Xiaoyi
>> *Subject:*Bug in SelectionDAG visitTargetIntrinsic
>> We ran into a problem where specifying IntrNoMem was causing our instruction
>> selection to fail with target specific intrinsics. After looking into the code
>> and ISel debug it looks like tablegen and SelectionDAG are using different
>> criteria to generate code for intrinsic_w_chain vs intrinsic_wo_chain.
>> In CodeGenDAGPatterns.cpp, tablegen decides based on whether IntrNoMem is set
>> or not. However with SelectionDAG, whether to use a chain or not is determined
>> by the call site attributes and not by the intrinsic.
>> So, we can get the situation where the call site has a different attribute
>> than the intrinsic, and this causes selection dag to fail.
>> I believe that this is wrong and that whether a chain should be generated or
>> not should come from only the intrinsic and not the call site. Since the
>> mapping of call -> intrinsic is by function name only, it should not matter if
>> the readnone attribute is set or not as that is irrelevant to the code
>> generator. Only what is set in the tablegen definition should be determine how
>> the intrinsic is generated.
>> So, I'm proposing the following patch. What this patch does is instead of
>> relying on the call site to determine if a chain is required, use instead the
>> read/write attributes of the intrinsic from the backend instead. There is not
>> much documentation on target intrinsics and no other backend uses them in this
>> manner.
>> This patch sound good?
>> Thanks,
>> Micah
>> <target_intrinsic_incorrect_chain.txt>_______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu <mailto:LLVMdev at cs.uiuc.edu>http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>