Undocumented Unconventional Order of BR Instruction Label Operands

[wu-benjamin]

Describe the Bug
The convention adopted by the syntax for the conditional 'br' instruction in the LLVM language reference manual (https://llvm.org/docs/LangRef.html#i-br) is:
br i1 <cond>, label <iftrue>, label <iffalse>

In Inkwell, an InstructionValue instr with opcode instr.get_opcode() == InstructionOpcode::Br and instr.get_num_operands() == 3 has the indices of the <iftrue> and <iffalse> operands swapped.

That is, instr.get_operand(1) represents the <iffalse> basic block and instr.get_operand(2) represents the <iftrue> basic block.

Expected Behavior
I expected the behavior to match the syntax for the conditional 'br' instruction in the LLVM language reference manual: instr.get_operand(1) represents the <iftrue> basic block and instr.get_operand(2) represents the <iffalse> basic block.

I don't think changing this to match the syntax for the conditional 'br' instruction in the LLVM language reference manual is feasible due to such a change being breaking. Instead, I propose documentation for Inkwell explicitly declares the semantics of each operand.

LLVM Version (please complete the following information):

• LLVM Version: 13.0.1
• Inkwell Branch Used: master

Desktop (please complete the following information):

• OS: MacOS (Ventura) 13.2.1 (22D68)

[TheDan64]

I don't follow... We supply cond, iftrue, and iffalse in that order:

inkwell/src/builder.rs

Lines 2213 to 2215 in 13c6c3b

	comparison.as_value_ref(),
	then_block.basic_block,
	else_block.basic_block,

[wu-benjamin]

Thanks for looking into this!

The build_conditional_branch implementation seems reasonable although I currently lack sufficient knowledge of llvm-sys to know if something happening in there is different from what I expect.

I've produced an example demonstrating the issue I see. Could you let me know if the pass/fail behaviour of the assertions in the example are expected?

Here's the small example illustrating the issue:

use inkwell::context::Context;

fn main () {
    let context = Context::create();
    let module = context.create_module("inkwell_br_operand_test");
    let builder = context.create_builder();

    let i32_type = context.i32_type();
    let arg_types = [i32_type.into()];
    let fn_type = i32_type.fn_type(&arg_types, false);
    let fn_value = module.add_function("inkwell_Br_operand_test", fn_type, None);

    let entry_bb = context.append_basic_block(fn_value, "entry_bb");
    let true_branch_bb = context.append_basic_block(fn_value, "true_branch_bb");
    let false_branch_bb = context.append_basic_block(fn_value, "false_branch_bb");

    let i32_arg = fn_value.get_first_param().unwrap();

    builder.position_at_end(entry_bb);
    let conditional_branch_instr = builder.build_conditional_branch(
        i32_arg.into_int_value(),
        true_branch_bb,
        false_branch_bb
    );
    
    println!("Conditional Branch Instruction:");
    println!("\n\tInstruction Value:");
    println!("\t\t{:?}", conditional_branch_instr);
    println!("\n\tOperand 0 (Expected Comparison):");
    println!("\t\t{:?}", conditional_branch_instr.get_operand(0).unwrap().left().unwrap());
    println!("\n\tOperand 1 (Expected True Branch BB):");
    println!("\t\t{:?}", conditional_branch_instr.get_operand(1).unwrap().right().unwrap());
    println!("\n\tOperand 2 (Expected False Branch BB):");
    println!("\t\t{:?}", conditional_branch_instr.get_operand(2).unwrap().right().unwrap());

    assert!(true_branch_bb == conditional_branch_instr.get_operand(2).unwrap().right().unwrap()); // PASSES BUT SHOULD FAIL
    assert!(false_branch_bb == conditional_branch_instr.get_operand(1).unwrap().right().unwrap()); // PASSES BUT SHOULD FAIL

    // assert!(true_branch_bb == conditional_branch_instr.get_operand(1).unwrap().right().unwrap()); // FAILS BUT SHOULD PASS
    // assert!(false_branch_bb == conditional_branch_instr.get_operand(2).unwrap().right().unwrap()); // FAILS BUT SHOULD PASS
}

Here's what is printed:

Conditional Branch Instruction:

	Instruction Value:
		InstructionValue { instruction_value: Value { name: "", address: 0x6000034b4060, is_const: false, is_null: false, is_undef: false, llvm_value: "  br i32 %0, label %true_branch_bb, label %false_branch_bb", llvm_type: "void" } }

	Operand 0 (Expected Comparison):
		IntValue(IntValue { int_value: Value { name: "", address: 0x600000ba5410, is_const: false, is_null: false, is_undef: false, llvm_value: "i32 %0", llvm_type: "i32" } })

	Operand 1 (Expected True Branch BB):
		BasicBlock { address: 0x6000010943c0, is_const: false, llvm_value: "\nfalse_branch_bb:                                  ; preds = %entry_bb\n", llvm_type: "label" }

	Operand 2 (Expected False Branch BB):
		BasicBlock { address: 0x600001094380, is_const: false, llvm_value: "\ntrue_branch_bb:                                   ; preds = %entry_bb\n", llvm_type: "label" }