Hi all, i'm writing an back-end for a new research processor architecture and have problems with the DAG Combiner. The processor architecture supports i1 and i32 registers. 1-bit registers are mainly used as comparison result but basic operations like OR are not possible between i1 registers. So I wrote custom lowering for i1 OR operations and replaced it by (trunc (or (aext x), (aext y))). Now the problem is that the DAG Combiner optimizes it back to an i1 OR operations that is not supported by the architecture. What is the best way to solve this problem? I take a look at the DAG Optimizer and for this OR operation it calls DAGCombiner::SimplifyBinOpWithSameOpcodeHands that folds (OP (aext x), (aext y)) -> (aext (OP x, y)). No check if the new operation is legal is performed. Kind regards Timo Stripf -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20090823/272a6e32/attachment.html>
On 23/08/2009, at 18.42, Stripf, Timo wrote:> Hi all, > > i’m writing an back-end for a new research processor architecture > and have problems with the DAG Combiner. The processor architecture > supports i1 and i32 registers. 1-bit registers are mainly used as > comparison result but basic operations like OR are not possible > between i1 registers. So I wrote custom lowering for i1 OR > operations and replaced it by (trunc (or (aext x), (aext y))). Now > the problem is that the DAG Combiner optimizes it back to an i1 OR > operations that is not supported by the architecture.The IA64 target has just been removed from the tree. It was the only target with legal i1 values, so there could be some problems. The Blackfin DSP can do simple i1 operations with the CC flag and status bits. Initially I also marked i1 as a legal type, but it caused a lot of problems. Now I pretend that the CC register can hold an i32. It just happens to always hold the values 0 and 1. The i1 logical operations are rarely needed, and they can be custom inserted when necessary, see BlackfinTargetLowering::LowerADDE(). I don't think you have to write custom lowering code to get the behaviour you want. Have you tried this: setOperationAction(ISD::OR, MVT::i1, Promote); If you can get your target to work with a legal i1 type, it would be great. The Blackfin target could use that as well.> What is the best way to solve this problem? I take a look at the > DAG Optimizer and for this OR operation it calls > DAGCombiner::SimplifyBinOpWithSameOpcodeHands that folds (OP (aext > x), (aext y)) -> (aext (OP x, y)). No check if the new operation is > legal is performed.When LegalOperations is set, the DAG combiner must not create illegal operations. It is a bug if it does. I recently fixed this in the first if statement in SimplifyBinOpWithSameOpcodeHands(). Perhaps you could add a check to the second if statement and submit a patch? /jakob
On 23/08/2009, at 19.12, Jakob Stoklund Olesen wrote:> When LegalOperations is set, the DAG combiner must not create illegal > operations. It is a bug if it does. I recently fixed this in the first > if statement in SimplifyBinOpWithSameOpcodeHands(). Perhaps you could > add a check to the second if statement and submit a patch?Sorry, the second 'if' is fine, I think. Do you have the patch from r78497? Are you still getting illegal ORs?
On Sun, Aug 23, 2009 at 9:42 AM, Stripf, Timo<Timo.Stripf at itiv.uni-karlsruhe.de> wrote:> Hi all, > > > > i’m writing an back-end for a new research processor architecture and have > problems with the DAG Combiner. The processor architecture supports i1 and > i32 registers. 1-bit registers are mainly used as comparison result but > basic operations like OR are not possible between i1 registers. So I wrote > custom lowering for i1 OR operations and replaced it by (trunc (or (aext x), > (aext y))). Now the problem is that the DAG Combiner optimizes it back to an > i1 OR operations that is not supported by the architecture. > > > > What is the best way to solve this problem? I take a look at the DAG > Optimizer and for this OR operation it calls > DAGCombiner::SimplifyBinOpWithSameOpcodeHands that folds (OP (aext x), (aext > y)) -> (aext (OP x, y)). No check if the new operation is legal is > performed.Sounds like a bug; the DAGCombiner isn't supposed to introduce illegal operations after operation legalization. -Eli
Hi Jakob, I forget to mention that I'm working atm on the old 2.5 release code base and not on the svn. So I don't know if the problem still exists. I'm going to test it now.> The Blackfin DSP can do simple i1 operations with the CC flag and > status bits. Initially I also marked i1 as a legal type, but it caused > a lot of problems. Now I pretend that the CC register can hold an i32. > It just happens to always hold the values 0 and 1. The i1 logical > operations are rarely needed, and they can be custom inserted when > necessary, see BlackfinTargetLowering::LowerADDE().I had also a lot of problems to get the i1 operations working. E.g. I had to override the getSetCCResultType to get is working and for ADDE/ADDC the i1 target registers are hardcoded. I'm writing the back-end to research the influence of several ISA characteristics on the processor performance. Large parts of my back-end are automatically generated by a general description. So I'm very interested in keeping the DAG as realistic as possible.> I don't think you have to write custom lowering code to get the > behaviour you want. Have you tried this: > > setOperationAction(ISD::OR, MVT::i1, Promote);I tried Promote and Expand but on the 2.5 code base it is not implemented.> If you can get your target to work with a legal i1 type, it would be > great. The Blackfin target could use that as well.Alright, I'll inform you if i get it running. I successfully compiled and simulated jpeg 2000 encoder/decoder before I ran into this problem.> When LegalOperations is set, the DAG combiner must not create illegal > operations. It is a bug if it does. I recently fixed this in the first > if statement in SimplifyBinOpWithSameOpcodeHands(). Perhaps you could > add a check to the second if statement and submit a patch?That sounds nice. I'll try out the svn code tomorrow. I think it'll take some days to converted the back-end to the new code base. -Timo -----Ursprüngliche Nachricht----- Von: Jakob Stoklund Olesen [mailto:stoklund at 2pi.dk] Gesendet: Sonntag, 23. August 2009 19:13 An: Stripf, Timo Cc: LLVM Developers Mailing List Betreff: Re: [LLVMdev] Problems with DAG Combiner On 23/08/2009, at 18.42, Stripf, Timo wrote:> Hi all, > > i'm writing an back-end for a new research processor architecture > and have problems with the DAG Combiner. The processor architecture > supports i1 and i32 registers. 1-bit registers are mainly used as > comparison result but basic operations like OR are not possible > between i1 registers. So I wrote custom lowering for i1 OR > operations and replaced it by (trunc (or (aext x), (aext y))). Now > the problem is that the DAG Combiner optimizes it back to an i1 OR > operations that is not supported by the architecture.The IA64 target has just been removed from the tree. It was the only target with legal i1 values, so there could be some problems. The Blackfin DSP can do simple i1 operations with the CC flag and status bits. Initially I also marked i1 as a legal type, but it caused a lot of problems. Now I pretend that the CC register can hold an i32. It just happens to always hold the values 0 and 1. The i1 logical operations are rarely needed, and they can be custom inserted when necessary, see BlackfinTargetLowering::LowerADDE(). I don't think you have to write custom lowering code to get the behaviour you want. Have you tried this: setOperationAction(ISD::OR, MVT::i1, Promote); If you can get your target to work with a legal i1 type, it would be great. The Blackfin target could use that as well.> What is the best way to solve this problem? I take a look at the > DAG Optimizer and for this OR operation it calls > DAGCombiner::SimplifyBinOpWithSameOpcodeHands that folds (OP (aext > x), (aext y)) -> (aext (OP x, y)). No check if the new operation is > legal is performed.When LegalOperations is set, the DAG combiner must not create illegal operations. It is a bug if it does. I recently fixed this in the first if statement in SimplifyBinOpWithSameOpcodeHands(). Perhaps you could add a check to the second if statement and submit a patch? /jakob