[PATCH][TargetInstrInfo] Fix the implementation of commuteInstruction to match the comment of the API

Tom Stellard tom at stellard.net
Thu May 8 16:13:05 PDT 2014


LGTM.

-Tom

On Thu, May 08, 2014 at 04:04:13PM -0700, Quentin Colombet wrote:
> Hi,
> 
> The proposed patch relaxes the behavior of TargetInstrInfo::commuteInstruction when TargetInstrInfo::findCommutedOpIndices returns false.
> 
> Currently TargetInstrInfo triggers a fatal error in such situation whereas based on the comment in the API[1] it should just return nullptr. Indeed the only precondition that should be ensured is that the instruction must be commutable.
> 
> Spotted via code inspection.
> 
> Is it okay to commit?
> 
> [1]
>   /// commuteInstruction - If a target has any instructions that are
>   /// commutable but require converting to different instructions or making
>   /// non-trivial changes to commute them, this method can overloaded to do
>   /// that.  The default implementation simply swaps the commutable operands.
>   /// If NewMI is false, MI is modified in place and returned; otherwise, a
>   /// new machine instruction is created and returned.  Do not call this
>   /// method for a non-commutable instruction, but there may be some cases
>   /// where this method fails and returns null.
> 
> Thanks,
> -Quentin

> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list