- User Since
- Jul 18 2017, 1:54 PM (104 w, 1 d)
Mon, Jul 15
Looks ok to me, but I would like some of the others to chime in first.
Wed, Jul 10
Tue, Jul 9
Mon, Jun 17
With cloned tests for FMF.
Jun 17 2019
There was an existing test, here it is with the modification for FMF
So I agree, but I think there should be a target specific combine with guards for Unsafe and nsz and this legalization case (for those who do not have it).
For your target. But as a catch all for legalization should do this.
We do this as a combine earlier in target specific code under this constraint. Matt, perhaps you just missing that combine in GlobalIsel?
I am saying you have it right Matt for using MI flags, but we should guard folding based on Unsafe or nsz in the Flags.
SDAG fails isNegatibleForFree if that constraint is not met and we do not translate.
Should we not also guard GlobslIsel translation in this case and avoid if not met?
Ok, with the constraint like in the DAG:
Seems reasonable to me.
Jun 14 2019
I was just about to change the test to assembler check test, not using the assert path...
Jun 3 2019
May 28 2019
May 1 2019
LGTM, thanks Sanjay
Apr 30 2019
Apr 18 2019
Feb 6 2019
Feb 5 2019
Migrated flag copy utility function to MachineInstr as a static function.
Feb 4 2019
Feb 1 2019
Jan 28 2019
Dec 18 2018
Dec 17 2018
Updated for posterity with the post review changes...
Dec 16 2018
As requested, add one more component for copy ir flags in to this review
Dec 13 2018
Oct 30 2018
Sep 19 2018
The fixed test for WebAssembly is in rL342598.
Ok, looking into it now.
Sep 13 2018
Changed copyIRFlags as suggested...
Sep 12 2018
Sep 11 2018
Collapsed for single version...
Hal/Sanjay could one of you have a look at this code, it would be great to wrap it up...
Sep 10 2018
All the nsw/nuw context is set above SDAG currently in code that is common to both GlobalIsel and SDAG, meaning adding them to MI should be analogous to SDAG as a pass through and for detection of already derived context for actions and side effects. Currently SDAG does have some DAG combining opts that utilize these flags. MI should too for GlobalIsel to map existing optimizations and functionality, else GlobalIsel will be handicapped. Having said that, if we do due diligence to map SDAG context in GlobalIsel, then there should be no new poison state, only that which currently exists. We should also keep the door open for new SDAG optimizations under nsw/nuw to map into that space to provide parity of support.
Sep 8 2018
The flags were requested for support in GlobalIsel, where there is a plan to make use of them once they are available. The path through SelectionDAG could also make use of the flags.
Sep 6 2018
With the appropriate test file...
I have a test case which will show shortly...
Sep 4 2018
I added one more non fmf instruction and there is room to add others if needed.
Thanks, that is a good alternative. I am testing it internally to see I get similar coverage. More later...
Aug 31 2018
In the newly created test case above, as checked in for test/CodeGen/X86/intersect-fma-fmf.ll, we get precisely this scenario. From a debug session breakpointed in llvm::SelectionDAGBuilder::visit at the line containing Node->intersectFlagsWith(IncomingFlags);
The Instruction in question is: %tmp11 = extractelement <4 x float> %tmp10, i32 1 as seen by the debugger, the Node we are associating it to is: t7: f32 = fadd nnan ninf nsz arcp contract afn reassoc t6, t5.
The extractelement is an Instruction that never carries flags and qualifies under
Another data point, this change caused no drift in the lit tests, meaning none of our FMF opts managed in the lit tests are modified by the behavior.
Correctly stated, but out of context. Some clarity first. The kinds of instructions in our case are neither strict nor fast, they never carry flags. Neither scalarization nor partial vectorization should cause FMF obliteration. Perhaps we need additional mechanisms?
Aug 30 2018
With the test file sync'd
I have a smaller test case now, its baseline analog should show up in the tree in a bit...
without the change:
I will have both with and without assembler example shortly, added here. As for the test case, it is pretty close to minimal with context needed for association, but I will see if I can shrink it a bit after the assembler is up.
Copying all available flags as false should not change the initial state of the object after its creation. It only does so because we are using AnyDefined as a sticky bit for assignment of any flag regardless of value. My notion is that any true value indicates that at least a value is set to something other than a default value (false). The test conditions occurs in partial vectorization and in our case, after scalarization on gpus where interior math operations occur. It happens frequently in our cases. The problem is when an instruction is associated to a Node that does have flags and where the Instruction it is associated to does not. This only ever occurs where the association is disjunct, meaning the Node and the Instruction originated differently. Generally its the other way around, where Node is empty and we are inheriting flags from an Instruction as the Instruction originated the Node in question. All other operations of intersect are done from Node to Node, ergo this change does not modify that behavior, that only occurs in this context.
Aug 22 2018
The attached test is a x86 approximation of internal tests, altered for platform, which show this problem which occurs frequently on many FMF optimization contexts.
Aug 20 2018
Actually the other test in rL339938 shows the positive side, where both binops are fast to show the intersection in /test/Transforms/LoopVectorize/reduction.ll
The select_meta test has a directly related case in it that maps to the bug test case in https://bugs.llvm.org/show_bug.cgi?id=38641