converting if else statement to ternary

Question

I have translated the following code using ternary. However, I knew there was something wrong with it. Can someone please point me into the right direction?

ForwardA = 0;
ForwardB = 0;

//EX Hazard
if (EXMEMRegWrite == 1) begin
 if (EXMEMrd != 0)
    if (EXMEMrd == IDEXrs)
        ForwardA = 2'b10;
   if (EXMEMrd == IDEXrt && IDEXTest == 0)
        ForwardB = 2'b10;
end


//MEM Hazard

if (MEMWBRegWrite == 1) begin
 if (MEMWBrd != 0) begin
    if (!(EXMEMRegWrite == 1 && EXMEMrd != 0 && (EXMEMrd == IDEXrs)))
            if (MEMWBrd == IDEXrs)
                ForwardA = 2'b01;
    if (IDEXTest == 0) begin
        if (!(EXMEMRegWrite == 1 && EXMEMrd != 0 && (EXMEMrd == IDEXrt)))
            if (MEMWBrd == IDEXrt)
                ForwardB = 2'b01;
    end
 end
end



ForwardA = (MEMWBRegWrite && MEMWBrd != 0 && (!(EXMEMRegWrite == 1 && EXMEMrd != 0 && (EXMEMrd == IDEXrs))) && (MEMWBrd == IDEXrs)) ?  
                                    2'b01 : ((EXMEMRegWrite && EXMEMrd != 0 && EXMEMrd == IDEXrs) ? 2'b10 : 0);

ForwardB = (IDEXTest == 0 && MEMWBRegWrite && MEMWBrd != 0 && (!(EXMEMRegWrite == 1 && EXMEMrd != 0 && (EXMEMrd == IDEXrt))) && (MEMWBrd == IDEXrs)) ?  
                                    2'b01 : ((EXMEMRegWrite && EXMEMrd != 0 && EXMEMrd == IDEXrt && IDEXTest == 0) ? 2'b10 : 0);

Answer 1

Surprisingly enough, I'm going to risk downvotes and tell you that the right direction is to leave your code in its relatively readable state.

I suspect the only thing you could do that would be worse would be to do it as a regular expression or convert it to inline assembly :-)

The fact that it's not converting easily should tell you something about the wisdom in what you're attempting.

---

Based on your comment elsewhere:

This is verilog and therefore I need to do it in ternary and can't have an if else, otherwise I would need an always block before and I don't want that... I want the remaining to be 0 if none of the conditions in the if else above is satisfied

Well, if you must do it, against my advice (and I'm not alone here in offering this advice), here's the method you should use (I have no idea what an "always block" even is so I'm not qualified to argue the point with you).

Since your current code is setting ForwardA and ForwardB to values then only changing them under certain conditions, you can transform that into a ternary by reversing the order. That's because, in your if version, later code takes precedence but earlier code takes precedence in the ternary.

Find out under what circumstances ForwardA and ForwardB are set in reverse order and reconstruct those conditions.

Here's your original code, compressed a bit. I've also changed your 2'b10 things into 2'b10' so we still get nice formatting in the SO rendering engine - don't forget to change them back.

ForwardA = 0;
ForwardB = 0;
if (EXMEMRegWrite == 1) begin
 if (EXMEMrd != 0)
    if (EXMEMrd == IDEXrs)
        ForwardA = 2'b10';
   if (EXMEMrd == IDEXrt && IDEXTest == 0)
        ForwardB = 2'b10';
end
if (MEMWBRegWrite == 1) begin
 if (MEMWBrd != 0) begin
    if (!(EXMEMRegWrite == 1 && EXMEMrd != 0 && (EXMEMrd == IDEXrs)))
            if (MEMWBrd == IDEXrs)
                ForwardA = 2'b01';
    if (IDEXTest == 0) begin
        if (!(EXMEMRegWrite == 1 && EXMEMrd != 0 && (EXMEMrd == IDEXrt)))
            if (MEMWBrd == IDEXrt)
                ForwardB = 2'b01';
    end
 end
end

You can see B is set in three places. It's set to 2'b01 in the bottom if, 2'b10 in the top one and 0 at the start. Converting the conditions:

ForwardB = ((MEMWBRegWrite == 1) &&
            (MEMWBrd != 0) &&
            (IDEXTest == 0) &&
            (!(EXMEMRegWrite == 1 && EXMEMrd != 0 && (EXMEMrd == IDEXrt))) &&
            (MEMWBrd == IDEXrt))
            ? 2'b01'
            : ((EXMEMRegWrite == 1) &&
               (EXMEMrd != 0) &&
               (EXMEMrd == IDEXrt && IDEXTest == 0))
               ? 2'b10'
               : 0;

Similarly for A:

ForwardA = ((MEMWBRegWrite == 1) &&
            (MEMWBrd != 0) &&
            (!(EXMEMRegWrite == 1 && EXMEMrd != 0 && (EXMEMrd == IDEXrs))) &&
            (MEMWBrd == IDEXrs))
            ? 2'b01'
            : ((EXMEMRegWrite == 1) &&
               (EXMEMrd != 0) &&
               (EXMEMrd == IDEXrs))
               ? 2'b10'
               : 0;

Now the theory behind that is good but I wouldn't be the least bit surprised if I'd made an error in the transcription, or if Verilog just threw its hands up in disgust, picked up its ball, and trotted off home :-)

Can I at least suggest, if you must follow this path, you both:

• try to leave the ternary expressions at least a little readable, with all that nice white space and multiple lines; and
• keep the original code in a comment so at least you can go back to it if you have problems or want to change the logic?

Seriously, you'll thank me in six months time when you're looking over this again, trying to figure out what on Earth you were thinking :-)

Answer 2

You don't need to do this. Stick the code in an 'always @*' block, and declare anything you're assigning to as 'reg'.

reg [1:0] ForwardA;
reg [1:0] ForwardB;

always @(*) begin
   // Your combo logic here..
end