[llvm-commits] [PATCH] Change return type of getOptimalMemOpType() from EVT to MVT.
Patrik Hägglund H
patrik.h.hagglund at ericsson.com
Wed Nov 28 02:27:55 PST 2012
Thanks for your review. I have now updated the patches. See below.
> The two patches also don't compile when applied together.
Sorry! A last-minute reordering.
> Should probably add a bitsGT to MVT for this part of the second patch. The trip through EVT will add checks for handling Extended types.
I just changed to:
- if (VT.bitsGT(LVT))
+ if (VT.getSizeInBits() > LVT.getSizeInBits())
> I don't realy like the intialization of VTSize being in the for loop header.
OK. VTSize is now put outside. (The intention was to limit the scope of VTSize.)
> I'm also not sure that's even safe to pull out of the loop since VT can change inside the loop. For the same reason, I dont' think you can subtract VTSize.
VTSize must follow (be set to the size of) VT. The patch didn't touch this. The patch only converted
while () {
[...]
Size -= VTSize;
}
into
for (;; Size -= VTSize) {
[...]
}
which are equivalent (C++03, section 6.5.3).
> I'm pretty sure this change isn't equivalent either because it allows MemOps to grow past the limit which it didn't previously do.
>
> - if (++NumMemOps > Limit)
> - return false;
> MemOps.push_back(VT);
> - Size -= VTSize;
> + if (MemOps.size() > Limit)
> + return false;
MemOps only differ if we return false. In that case MemOps is not used. Therefore, this should be safe.
Here are the updated set of patches. Besides addressing your comments, the loop simplification is splitted into a third patch, and remade to update VTSize at only a single place. VTSize >>= 1, which is taken from the assumption that MVT integer types are a straight series of power-of-two sized integers, is replaced with the generic VT.getSizeInBits() / 8. (In our target we have registers which are not power-of-two sized.)
Any comments?
[PATCH 1/3] Cleanup: Change return type of getOptimalMemOpType()
from EVT to MVT.
The user of this function (FindOptimalMemOpLowering) asserts if not
receiving an MVT. Therefore, it makes no sense to use EVT.
---
include/llvm/Target/TargetLowering.h | 4 ++--
lib/Target/ARM/ARMISelLowering.cpp | 2 +-
lib/Target/ARM/ARMISelLowering.h | 2 +-
lib/Target/Mips/MipsISelLowering.cpp | 2 +-
lib/Target/Mips/MipsISelLowering.h | 2 +-
lib/Target/PowerPC/PPCISelLowering.cpp | 4 ++--
lib/Target/PowerPC/PPCISelLowering.h | 4 ++--
lib/Target/X86/X86ISelLowering.cpp | 4 ++--
lib/Target/X86/X86ISelLowering.h | 4 ++--
[PATCH 2/3] Cleanup: Use an MVT instead of EVT.
The code asserts at VT.getSimpleVT() if VT isn't an MVT.
---
lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 8 ++++----
[PATCH 3/3] Simplification of loop logic in FindOptimalMemOpLowering.
---
lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 14 +++++---------
/Patrik Hägglund
From: Craig Topper [mailto:craig.topper at gmail.com]
Sent: den 25 november 2012 04:21
To: Patrik Hägglund H
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] [PATCH] Change return type of getOptimalMemOpType() from EVT to MVT.
The two patches also don't compile when applied together.
On Sat, Nov 24, 2012 at 7:05 PM, Craig Topper <craig.topper at gmail.com> wrote:
Should probably add a bitsGT to MVT for this part of the second patch. The trip through EVT will add checks for handling Extended types.
LVT = (MVT::SimpleValueType)(LVT.SimpleTy - 1);
assert(LVT.isInteger());
- if (VT.bitsGT(LVT))
+ if (EVT(VT).bitsGT(LVT))
VT = LVT;
}
I don't realy like the intialization of VTSize being in the for loop header. It makes it confusing since VTSize isn't the variable being used for iteration. I'm also not sure that's even safe to pull out of the loop since VT can change inside the loop. For the same reason, I dont' think you can subtract VTSize.
- unsigned NumMemOps = 0;
- while (Size != 0) {
- unsigned VTSize = VT.getSizeInBits() / 8;
+ for (unsigned VTSize = VT.getSizeInBits() / 8; Size != 0; Size -= VTSize) {
I'm pretty sure this change isn't equivalent either because it allows MemOps to grow past the limit which it didn't previously do.
- if (++NumMemOps > Limit)
- return false;
MemOps.push_back(VT);
- Size -= VTSize;
+ if (MemOps.size() > Limit)
+ return false;
On Fri, Nov 23, 2012 at 3:44 AM, Patrik Hägglund H <patrik.h.hagglund at ericsson.com> wrote:
Here are two small patches that change the return type of getOptimalMemOpType()
from EVT to MVT. In the second patch there is also some other minor cleanups.
Let me know if you have any objections. Otherwise, I will commit them.
Regards,
Patrik Hägglund
[PATCH 1/2] Cleanup: Change return type of getOptimalMemOpType()
from EVT to MVT.
The user of this function (FindOptimalMemOpLowering) asserts if not
receiving an MVT. Therefore, it makes no sense to use EVT.
---
include/llvm/Target/TargetLowering.h | 4 ++--
lib/Target/ARM/ARMISelLowering.cpp | 2 +-
lib/Target/ARM/ARMISelLowering.h | 2 +-
lib/Target/Mips/MipsISelLowering.cpp | 2 +-
lib/Target/Mips/MipsISelLowering.h | 2 +-
lib/Target/PowerPC/PPCISelLowering.cpp | 4 ++--
lib/Target/PowerPC/PPCISelLowering.h | 4 ++--
lib/Target/X86/X86ISelLowering.cpp | 4 ++--
lib/Target/X86/X86ISelLowering.h | 4 ++--
[PATCH 2/2] Cleanup: Use an MVT instead of EVT. Clean up loop logic.
The code asserts at VT.getSimpleVT() if VT isn't an MVT.
---
lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 19 ++++++++-----------
_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
--
~Craig
--
~Craig
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Cleanup-Change-return-type-of-getOptimalMemOpType-fr.patch
Type: application/octet-stream
Size: 7883 bytes
Desc: 0001-Cleanup-Change-return-type-of-getOptimalMemOpType-fr.patch
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121128/2a449b6e/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Cleanup-Use-an-MVT-instead-of-EVT.patch
Type: application/octet-stream
Size: 1897 bytes
Desc: 0002-Cleanup-Use-an-MVT-instead-of-EVT.patch
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121128/2a449b6e/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Small-simplification-of-loop-logic.patch
Type: application/octet-stream
Size: 1465 bytes
Desc: 0003-Small-simplification-of-loop-logic.patch
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121128/2a449b6e/attachment-0002.obj>
More information about the llvm-commits
mailing list