Re[2]: [PATCH] Passing down BinaryOperator flags to BinarySDNode + X86 optimization

[Данил Трошков]

 1)
     (SubclassData & ~NUW) | (b * NUW);
Maybe better to write somehow
(SubclassData & ~NUW) | (b ? NUW : 0);
IMHO it is more readable

2) const BinarySDNode *BinNode = static_cast<const BinarySDNode*>(N);
formatting: \t...
+                BinNode->hasNoUnsignedWrap(),
+         BinNode->hasNoSignedWrap(),
+         BinNode->isExact());
the same...
+                    bool nsw, bool exact) {
+          bool nsw, bool exact) {
+        Op1, Op2, HasNUW, HasNSW, IsExact);
+ if (BinNode->hasNoSignedWrap())
+   break;

3) Maybe better to remove bundle of cases on low level?
In order to reduce duplication of code...



Sat, 31 May 2014 19:30:24 -0700 от Marcello Maggioni <hayarms at gmail.com>:
>Here an updated patch.
>
>The changes are:
>
>- I stripped away the new getBinaryNode function and integrated the functionality in the getNode() function with two operands.
>- I removed the refactoring for the binary folding logic. Removing getBinaryNode() makes putting that logic in an external function not needed. (this also fixes the clang-format problem Owen pointed out).
>- Cleaned up some stuff and fixed some style problems.
>- Integrated the changes Pete pointed out:
>  - Added the flags to the NodeID construction
>  - Created a test case for the CSE (not sure it is the best test though ... I'm not an expert testing this kind of stuff)
>  - Merged the test for the performance optimization in the optimization patch.
>  - Integrated the code cleanups that were specified.
>  - Reduced the test case for the x86 opt.
>
>Now as a result there are only two patches:
>  - flag_nodes.patch
>  - x86_opt_wrap.patch
>
>I didn't changed BinarySDNode to have classof(), because I think is probably better doing it separately , but also because it is kind of more complex than it seems , because it is not clear exactly what are the opcodes that become BinarySDNodes (a part from the obvious binary ADD, SUB ... etc operations).
>It is probably necessary to take more time to collect an accurate list of all the opcodes that always end up being lowered as BinarySDNodes and add the classof() then.
>
>Marcello
>
>
>_______________________________________________
>llvm-commits mailing list
>llvm-commits at cs.uiuc.edu
>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140602/3429da02/attachment.html>