[PATCH] propagate IR-level fast-math-flags to DAG nodes
Andrea Di Biagio
Andrea_DiBiagio at sn.scee.net
Thu Apr 9 04:52:21 PDT 2015
Hi Sanjay,
I only had a couple of minor comments/questions. In general I think this is a nice initial patch.
If I remember correctly, the initial support for nsw/nuw/exact flags in SelectionDAG was added by Marcello Maggioni. In case, I suggest you to add him in cc to the code review (I don't know if he is still working on this..).
================
Comment at: include/llvm/CodeGen/SelectionDAG.h:1230-1231
@@ -1230,5 +1229,4 @@
- BinarySDNode *GetBinarySDNode(unsigned Opcode, SDLoc DL, SDVTList VTs,
- SDValue N1, SDValue N2, bool nuw, bool nsw,
- bool exact);
+ SDNode *GetSDNodeWithFlags(unsigned Opcode, SDLoc DL, SDVTList VTs,
+ ArrayRef<SDValue> Ops, const SDNodeFlags *Flags);
----------------
My understanding is that flags can only be present on binary operations. This is also the reason why originally nsw/nuw/exact were only added to BinarySDNode.
Is there a reason why BinarySDNode should extend from SDNode? At the moment, 'Ops' is always expected to have two SDValues.
================
Comment at: include/llvm/CodeGen/SelectionDAGNodes.h:971-977
@@ +970,9 @@
+
+ void setHasNoUnsignedWrap(bool b) {
+ Flags = (Flags & ~NoUnsignedWrap) | (b ? NoUnsignedWrap : 0);
+ }
+
+ void setHasNoSignedWrap(bool b) {
+ Flags = (Flags & ~NoSignedWrap) | (b ? NoSignedWrap : 0);
+ }
+
----------------
This is ok. However, what about having something like this?
```
setNoUnsignedWrap() {
Flags |= NoUnsignedWrap;
}
clearNoUnsignedWrap() {
Flags ^= NoUnsignedWrap;
}
```
Also it is a shame that most of this code is repeated in 'class SDNodeWithFlags'. I wonder if there is a better design that allows to delegate the 'bit manipulation part' as much as possible to 'SDNodeFlags'...
http://reviews.llvm.org/D8900
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list