search for: dagcombine

Right. DAGCombine will insert *illegal* nodes before legalize. Evan On Jan 19, 2009, at 8:17 PM, Eli Friedman wrote: > On Mon, Jan 19, 2009 at 6:36 PM, Scott Michel <scottm at aero.org> wrote: >> I just ran across something interesting: DAGCombine inserts a 64-bit >> constant as the result of...

I just ran across something interesting: DAGCombine inserts a 64-bit constant as the result of converting a (bitconvert (fabs val)) to a (and (bitconvert val), i64const). The problem: i64 constants have to be legalized for the CellSPU platform. DAGCombine is doing the right thing but it's not doing the right thing for CellSPU and it'...

Duncan: DAGCombine is inserting an IllegalOperation after target-specific instruction legalization has occurred. I'm inserting the fabs and the bitconvert during instruction legalization; DAGCombine is converting the fabs/bitconvert to an 'and' on its second (third?) pass. -scooter On Jan 20, 200...

Yes, it was I who put that rant in the commit log and it's justified. Worse, it's unreasonable to actually go through all of DAGCombiner's code and check to see if certain kinds of constants, e.g., i64, are legal during a particular phase of DAGCombiner. DAGCombiner does good work and the backends are supposed to be good citizens. CellSPU is certainly trying to be a good citizen, no matter how frustrating that becomes on certai...

Evan: And after legalize too. DAGCombine gets run after legalization. :-) -scooter On Jan 19, 2009, at 10:52 PM, Evan Cheng wrote: > Right. DAGCombine will insert *illegal* nodes before legalize. > > Evan > > On Jan 19, 2009, at 8:17 PM, Eli Friedman wrote: > >> On Mon, Jan 19, 2009 at 6:36 PM, Scott Michel &lt...

Hi all, The DAGCombiner pass actually runs even if the optimize level is set to None. This can result in incorrect debug information or unexpected stepping/debugging experience. Not to mention that having good stepping/debugging experience is the major reason to compile at /O0. I recently suggested a patch to disable on...

Hello, In the combine 2 step (after legalization), in the DAGCombiner::visitBIT_CONVERT() method, the DAG combiner is replacing an FABS followed by a BIT_CONVERT, to a BIT_CONVERT followed by an AND 0x7FFFFFFFFFFFFFFF. Everything is 64 bit. On my target, FABS and BIT_CONVERT are legal in 64 bit, but AND in not legal in 64 bit (is declared custom). So the dag combine...

...me of your points below seem to be contradictory. The advice to use target-independent nodes when feasible seems sound to me, so I wrote up a comment about it in SelectionDAGNodes.h. If you can formulate your thoughts in the form of specific documentation changes, that would be helpful. In theory, DAGCombiner is supposed to check if an operation is legal before using it, when it is running after legalize. It seems in practice it often doesn't do this. Our usual response when people hit this problem is "then fix it". Are you saying that the extent of the problem makes this infeasible? Dan...

...her than treat each load and store conservatively, which takes O(10) instructions. My target's allowsUnalignedMemoryOperations() always returns 'false', and the setOperationAction()s for i8,i16,i32 loads and stores are all 'Custom'. I'm running into a problem where DAGCombiner is being too clever for me; it runs LegalizeDAG, which calls my custom LowerLOAD() and LowerSTORE() routines (which emit between 1 and O(10) SDValues, depending on alignment information), and then runs DAGCombine. To lower an i16 STORE that is known to be in the high-addressed 2 bytes of a wo...

On Mon, Jan 19, 2009 at 6:36 PM, Scott Michel <scottm at aero.org> wrote: > I just ran across something interesting: DAGCombine inserts a 64-bit > constant as the result of converting a (bitconvert (fabs val)) to a > (and (bitconvert val), i64const). > > The problem: i64 constants have to be legalized for the CellSPU > platform. DAGCombine is doing the right thing but it's not doing the > right thing f...

new warnings, I think lib/CodeGen/SelectionDAG/DAGCombiner.cpp: In member function ‘llvm::SDValue<unnamed>::DAGCombiner::FindBetterChain(llvm::SDNode*, llvm::SDValue)’: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:6006: warning: ‘SrcValueOffset’ may be used uninitialized in this function lib/CodeGen/SelectionDAG/DAGCombiner.cpp:6006: note: ‘SrcVal...

We discovered what is happening. SDAGCombiner essentially looks at various combinations of nodes to do with vectors, and when it can, it creates a vector shuffle. The problem is, that our vector shuffle lowering builds new trees with vector element, or vector sub-vector insert sequences. The generic DAGCombiner, reconstructs these into a ne...

Hi Gabriele, > In the combine 2 step (after legalization), in the DAGCombiner::visitBIT_CONVERT() method, the DAG combiner is replacing an FABS followed by a BIT_CONVERT, to a BIT_CONVERT followed by an AND 0x7FFFFFFFFFFFFFFF. Everything is 64 bit. > On my target, FABS and BIT_CONVERT are legal in 64 bit, but AND in not legal in 64 bit (is declared custom). So the dag co...

> Historically nodes marked "custom" were considered legal, so the > DAGCombiner would have been correct to generate it. Not sure how > that ever worked though. I think Dan split the isOperationLegal > method into isOperationLegal and isOperationLegalOrCustom for reasons > related to this kind of thing. I don't know whether the DAGCombiner > is now only supp...

On Tuesday 20 January 2009 07:52:37 Evan Cheng wrote: > Right. DAGCombine will insert *illegal* nodes before legalize. There are two stages of legalization: legalization of types, followed by legalization of operations. Before type legalization DAGCombine is allowed to create nodes with illegal types and illegal operations. After type legalization but before operation...

...truction selection than DAGCombining. Try movign it to <Target>ISelDAGToDAG's Select (or potentially PreprocessISelDAG). Th -Nirav On Tue, Mar 6, 2018 at 4:05 PM Martin J. O'Riordan <MartinO at theheart.ie> wrote: > We discovered what is happening. > > > > SDAGCombiner essentially looks at various combinations of nodes to do with > vectors, and when it can, it creates a vector shuffle. The problem is, > that our vector shuffle lowering builds new trees with vector element, or > vector sub-vector insert sequences. The generic DAGCombiner, reconstructs...

Hi Gabrielle, > > Historically nodes marked "custom" were considered legal, so the > > DAGCombiner would have been correct to generate it. Not sure how > > that ever worked though. I think Dan split the isOperationLegal > > method into isOperationLegal and isOperationLegalOrCustom for reasons > > related to this kind of thing. I don't know whether the DAGCombiner > &...

...en bit casts will not be free. > The attached patch demonstrates that, but is missing the proper check to know what DAG combine will do (see TODO). > > I think you're approaching this backwards: the obvious thing to do is to generate the insert_vector_elt sequence unconditionally, and DAGCombine that sequence to a build_vector when appropriate. Hi Eli, I have started to look into the direction you gave me. I may have miss something but I do not see how the proposed direction solves the issue. Indeed to be able to DAGCombine a insert_vector_elt sequences into a ARMISD::build_vector, I sti...

Is this piece of code in DAGCombiner::visitLOAD removing a dead node? 06155 if (N->use_empty()) { 06156 removeFromWorkList(N); 06157 DAG.DeleteNode(N); 06158 } If it is, is there a reason it doesn't push its operands to the work list as done in line 974-975? 00970 // If N has no uses,...

On Wednesday 18 February 2009 21:43, Dan Gohman wrote: > I agree, that doesn't look right. It looks like this > is what was intended: > > Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp > =================================================================== > --- lib/CodeGen/SelectionDAG/DAGCombiner.cpp (revision 65000) > +++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp (working copy) > @@ -4903,9 +4903,9 @@ > // resultant store does not need a higher alignment th...