softdevteam / ykrustc

Yorick Meta-tracer
Other
6 stars 4 forks source link

Insertion point caching might be incorrect. #86

Closed vext01 closed 3 years ago

vext01 commented 4 years ago

In the SirCx, we cache each LLVM Builder's insertion point in a Hashmap: https://github.com/softdevteam/ykrustc/blob/043bf15b2c4fd0b21c02f6fe3d82bb8f663faf75/src/librustc/sir.rs#L62

The insertion point is a <block, instr_idx> pair.

Whilst I was implementing instruction insertion, I encountered the following code in the LLVM Builder:

    fn alloca(&mut self, ty: &'ll Type, align: Align) -> &'ll Value {
        let mut bx = Builder::with_cx(self.cx);
        bx.position_at_start(unsafe { llvm::LLVMGetFirstBasicBlock(self.llfn()) });
        bx.dynamic_alloca(ty, align)
    }

    fn dynamic_alloca(&mut self, ty: &'ll Type, align: Align) -> &'ll Value {
        unsafe {
            let alloca = llvm::LLVMBuildAlloca(self.llbuilder, ty, UNNAMED);
            llvm::LLVMSetAlignment(alloca, align.bytes() as c_uint);
            alloca
        }
    }

When emitting an alloca() instruction, a new builder is first created and the insertion point is set to the start of the first block in the function. This is because allocas must be the first instructions in a function.

If there are already existing builders for that block, won't their insertion point be skewed?

I think we may have to cache the instruction pointers and express the insertion point as a <block, instruction> pair.

@ptersilie I think we should Jitsi when you are back.