llvm dev - Jul 2012 - [LLVMdev] TLI.getSetCCResultType() and/or MVT broken by design?

if (N0.getOpcode() == ISD::SETCC
      && (LegalOperations
      || (!LegalOperations && VT.isPow2VectorType())))

But the comment right after it is:
    // sext(setcc) -> sext_in_reg(vsetcc) for vectors.
    // Only do this before legalize for now.
    if (VT.isVector() && !LegalOperations) {

So, these optimizations are never safe in the general case if we can't
guarantee that TLI.getSetCCResultType() returns a valid type.

From: Rotem, Nadav [mailto:nadav.rotem at intel.com]
Sent: Friday, July 27, 2012 12:25 PM
To: Villmow, Micah; Developers Mailing List
Subject: RE: TLI.getSetCCResultType() and/or MVT broken by design?

Hi Micah,

I think that getSetCCResultType should only be called for legal types. Disabling
it on isPow2VectorType is not the way to go because there are other illegal
vector types which are pow-of-two. I suggest that you call it only after
type-legalization.
BTW, you can't set the LLVMTy yourself because you don't have access to
the LLVMContext  at that point.

Nadav

From: llvmdev-bounces at cs.uiuc.edu<mailto:llvmdev-bounces at
cs.uiuc.edu> [mailto:llvmdev-bounces at cs.uiuc.edu] On Behalf Of Villmow,
Micah
Sent: Friday, July 27, 2012 21:52
To: Developers Mailing List
Subject: [LLVMdev] TLI.getSetCCResultType() and/or MVT broken by design?

I'm running into lots of problems with this call back. Mostly the problem
occurs because this callback is used before types are legalized. However, the
code generator does not have a 1-1 correspondence between all LLVM types and the
codegen types. This leads to problems when getSetCCResultType is passed in an
invalid type, but has a valid LLVM type attached to it. An example is <3 x
float>. getSetCCResultType can return any type, and in the AMDIL backends
case, for a <3 x float>, returns the corresponding integer version of the
vector. The problem comes in code like the following(comments removed):
This is from DAGCombiner.cpp:visitSIGN_EXTEND.
   EVT N0VT = N0.getOperand(0).getValueType();
...
      EVT SVT = TLI.getSetCCResultType(N0VT);
...
      if (VT.getSizeInBits() == SVT.getSizeInBits())
        return DAG.getSetCC(N->getDebugLoc(), VT, N0.getOperand(0),
                             N0.getOperand(1),
                            
cast<CondCodeSDNode>(N0.getOperand(2))->get());

SVT.getSizeInBits() crashes, because TLI.getSetCCResultType returns an invalid
MVT type and LLVMTy is NULL. Since there is no way to specify the LLVMTy
manually, there is no way to fix this without finding all of the locations that
use this and disabling them.

I'm disabling via VT.isPow2VectorType() because an extra check, but it seems
like this isn't preferable.  So should I conditionalize the pre-liegalized
check, or allow a backend to set the LLVMTy of newly created MVT types.

So, is the design to disallow backends to set this broken, or what is expected?
Let me know what you think is the best way forward.

Thanks,
Micah
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120727/d6be7f61/attachment.html>

We no longer have vsetcc, so the comment is wrong.  The code looks incorrect.
The fact that a vector is power-of-two does not guarantee anything about its
legality. For example <128 x i64> would pass the condition in the code
below, and die on most targets.


From: Villmow, Micah [mailto:Micah.Villmow at amd.com]
Sent: Friday, July 27, 2012 22:33
To: Rotem, Nadav; Developers Mailing List
Subject: RE: TLI.getSetCCResultType() and/or MVT broken by design?

  if (N0.getOpcode() == ISD::SETCC
      && (LegalOperations
      || (!LegalOperations && VT.isPow2VectorType())))

But the comment right after it is:
    // sext(setcc) -> sext_in_reg(vsetcc) for vectors.
    // Only do this before legalize for now.
    if (VT.isVector() && !LegalOperations) {

So, these optimizations are never safe in the general case if we can't
guarantee that TLI.getSetCCResultType() returns a valid type.

From: Rotem, Nadav [mailto:nadav.rotem at
intel.com]<mailto:[mailto:nadav.rotem at intel.com]>
Sent: Friday, July 27, 2012 12:25 PM
To: Villmow, Micah; Developers Mailing List
Subject: RE: TLI.getSetCCResultType() and/or MVT broken by design?

Hi Micah,

I think that getSetCCResultType should only be called for legal types. Disabling
it on isPow2VectorType is not the way to go because there are other illegal
vector types which are pow-of-two. I suggest that you call it only after
type-legalization.
BTW, you can't set the LLVMTy yourself because you don't have access to
the LLVMContext  at that point.

Nadav

From: llvmdev-bounces at cs.uiuc.edu<mailto:llvmdev-bounces at
cs.uiuc.edu> [mailto:llvmdev-bounces at cs.uiuc.edu] On Behalf Of Villmow,
Micah
Sent: Friday, July 27, 2012 21:52
To: Developers Mailing List
Subject: [LLVMdev] TLI.getSetCCResultType() and/or MVT broken by design?

I'm running into lots of problems with this call back. Mostly the problem
occurs because this callback is used before types are legalized. However, the
code generator does not have a 1-1 correspondence between all LLVM types and the
codegen types. This leads to problems when getSetCCResultType is passed in an
invalid type, but has a valid LLVM type attached to it. An example is <3 x
float>. getSetCCResultType can return any type, and in the AMDIL backends
case, for a <3 x float>, returns the corresponding integer version of the
vector. The problem comes in code like the following(comments removed):
This is from DAGCombiner.cpp:visitSIGN_EXTEND.
   EVT N0VT = N0.getOperand(0).getValueType();
...
      EVT SVT = TLI.getSetCCResultType(N0VT);
...
      if (VT.getSizeInBits() == SVT.getSizeInBits())
        return DAG.getSetCC(N->getDebugLoc(), VT, N0.getOperand(0),
                             N0.getOperand(1),
                            
cast<CondCodeSDNode>(N0.getOperand(2))->get());

SVT.getSizeInBits() crashes, because TLI.getSetCCResultType returns an invalid
MVT type and LLVMTy is NULL. Since there is no way to specify the LLVMTy
manually, there is no way to fix this without finding all of the locations that
use this and disabling them.

I'm disabling via VT.isPow2VectorType() because an extra check, but it seems
like this isn't preferable.  So should I conditionalize the pre-liegalized
check, or allow a backend to set the LLVMTy of newly created MVT types.

So, is the design to disallow backends to set this broken, or what is expected?
Let me know what you think is the best way forward.

Thanks,
Micah
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120727/8666f15d/attachment.html>

Yep. So I'm guessing getSetCCResultType should never be called with an
invalid type. This might not be the only spot that this happens, just the one
that I happen to be tripping up on.

Micah

From: Rotem, Nadav [mailto:nadav.rotem at intel.com]
Sent: Friday, July 27, 2012 12:42 PM
To: Villmow, Micah; Developers Mailing List
Subject: RE: TLI.getSetCCResultType() and/or MVT broken by design?

We no longer have vsetcc, so the comment is wrong.  The code looks incorrect.
The fact that a vector is power-of-two does not guarantee anything about its
legality. For example <128 x i64> would pass the condition in the code
below, and die on most targets.


From: Villmow, Micah [mailto:Micah.Villmow at amd.com]
Sent: Friday, July 27, 2012 22:33
To: Rotem, Nadav; Developers Mailing List
Subject: RE: TLI.getSetCCResultType() and/or MVT broken by design?

  if (N0.getOpcode() == ISD::SETCC
      && (LegalOperations
      || (!LegalOperations && VT.isPow2VectorType())))

But the comment right after it is:
    // sext(setcc) -> sext_in_reg(vsetcc) for vectors.
    // Only do this before legalize for now.
    if (VT.isVector() && !LegalOperations) {

So, these optimizations are never safe in the general case if we can't
guarantee that TLI.getSetCCResultType() returns a valid type.

From: Rotem, Nadav [mailto:nadav.rotem at
intel.com]<mailto:[mailto:nadav.rotem at intel.com]>
Sent: Friday, July 27, 2012 12:25 PM
To: Villmow, Micah; Developers Mailing List
Subject: RE: TLI.getSetCCResultType() and/or MVT broken by design?

Hi Micah,

I think that getSetCCResultType should only be called for legal types. Disabling
it on isPow2VectorType is not the way to go because there are other illegal
vector types which are pow-of-two. I suggest that you call it only after
type-legalization.
BTW, you can't set the LLVMTy yourself because you don't have access to
the LLVMContext  at that point.

Nadav

From: llvmdev-bounces at cs.uiuc.edu<mailto:llvmdev-bounces at
cs.uiuc.edu> [mailto:llvmdev-bounces at cs.uiuc.edu] On Behalf Of Villmow,
Micah
Sent: Friday, July 27, 2012 21:52
To: Developers Mailing List
Subject: [LLVMdev] TLI.getSetCCResultType() and/or MVT broken by design?

I'm running into lots of problems with this call back. Mostly the problem
occurs because this callback is used before types are legalized. However, the
code generator does not have a 1-1 correspondence between all LLVM types and the
codegen types. This leads to problems when getSetCCResultType is passed in an
invalid type, but has a valid LLVM type attached to it. An example is <3 x
float>. getSetCCResultType can return any type, and in the AMDIL backends
case, for a <3 x float>, returns the corresponding integer version of the
vector. The problem comes in code like the following(comments removed):
This is from DAGCombiner.cpp:visitSIGN_EXTEND.
   EVT N0VT = N0.getOperand(0).getValueType();
...
      EVT SVT = TLI.getSetCCResultType(N0VT);
...
      if (VT.getSizeInBits() == SVT.getSizeInBits())
        return DAG.getSetCC(N->getDebugLoc(), VT, N0.getOperand(0),
                             N0.getOperand(1),
                            
cast<CondCodeSDNode>(N0.getOperand(2))->get());

SVT.getSizeInBits() crashes, because TLI.getSetCCResultType returns an invalid
MVT type and LLVMTy is NULL. Since there is no way to specify the LLVMTy
manually, there is no way to fix this without finding all of the locations that
use this and disabling them.

I'm disabling via VT.isPow2VectorType() because an extra check, but it seems
like this isn't preferable.  So should I conditionalize the pre-liegalized
check, or allow a backend to set the LLVMTy of newly created MVT types.

So, is the design to disallow backends to set this broken, or what is expected?
Let me know what you think is the best way forward.

Thanks,
Micah
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120727/222926bf/attachment.html>