[LLVMbugs] [Bug 19268] New: The way BasicTTI uses TargetLoweringBase::isOperationXXX does not match the way the information is set

[bugzilla-daemon at llvm.org]

http://llvm.org/bugs/show_bug.cgi?id=19268

            Bug ID: 19268
           Summary: The way BasicTTI uses
                    TargetLoweringBase::isOperationXXX does not match the
                    way the information is set
           Product: libraries
           Version: trunk
          Hardware: PC
                OS: All
            Status: NEW
          Severity: normal
          Priority: P
         Component: Common Code Generator Code
          Assignee: unassignedbugs at nondot.org
          Reporter: qcolombet at apple.com
                CC: llvmbugs at cs.uiuc.edu
    Classification: Unclassified

While working on the vectorizer cost model, I’ve noticed that BasicTTI does not
query TLI->isOperationLegalXXX with the same type as the actions were set by
the target (setOperationAction).

Because of that, the transformations may take bad decisions.
For instance, the vectorizer may assume that the scalarization overhead is
cheap because BasicTTI thinks some stuff are legal whereas they are not.

r204884 is an example where we workaround this problem.

For a more detailed example:
X86 backend defines a bunch of setOpertionAction like this:
X86TargetLowering::resetOperationActions:
 if (Subtarget->is64Bit()) {
    setOperationAction(ISD::UINT_TO_FP     , MVT::i32  , Promote);
    setOperationAction(ISD::UINT_TO_FP     , MVT::i64  , Custom);    // <—
Custom, given type: source type.
  }
[...]
  if (Subtarget->is64Bit()) {
    setOperationAction(ISD::FP_TO_UINT     , MVT::i64  , Expand); // <— Expand,
given type: destination type.
    setOperationAction(ISD::FP_TO_UINT     , MVT::i32  , Promote);
  } 


BasicTTI is using the TLI like this:
 BasicTTI::getCastInstrCost:
  // If the cast is marked as legal (or promote) then assume low cost.
  if (TLI->isOperationLegalOrPromote(ISD, DstLT.second))       // <— Given
type: destination type.
    return 1;

This does not match the definition and in particular, any call with UINT_TO_FP
will query with the wrong type (the destination instead of the source type) and
likely end up with the wrong assumption that this is legal.

The proper fix would likely to have a lookup table to know which type should be
used for the query. This lookup table should be compatible with the way
setOperationAction method is used (and maybe shared?).

VectorLegalizer::LegalizeOp is already doing this kind of lookup:
  case ISD::FP_TO_SINT:
  case ISD::FP_TO_UINT:
[...]
    QueryType = Node->getValueType(0);
    break;
[…]
  case ISD::SINT_TO_FP:
  case ISD::UINT_TO_FP:
    QueryType = Node->getOperand(0).getValueType();
    break;

We should share this kind of logic somewhere!

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20140327/19676fca/attachment.html>