Skip to content

Add builder methods for alignable instructions to add alignment without converting to an InstructionValue. #600

@ErisianArchitect

Description

@ErisianArchitect

Is your feature request related to a problem? Please describe.
I found that I wanted to be able to set the alignment on alloca/store/load instructions (and any other instruction that allows setting the alignment), and I thought it would be nice to either have an optional alignment argument, or to have a separate function for setting the alignment while building the instruction.

Describe the solution you'd like
Here's an example that I wrote up of the build_alloca method but with alignment:

    pub fn build_aligned_alloca<T: BasicType<'ctx>>(
        &self,
        ty: T,
        name: &str,
        alignment: u32,
    ) -> Result<PointerValue<'ctx>, BuilderError> {
        if !is_alignment_ok(alignment) {
            return Err(BuilderError::AlignmentError(AlignmentError::NonPowerOfTwo(alignment)));
        }
        if self.positioned.get() != PositionState::Set {
            return Err(BuilderError::UnsetPosition);
        }
        let c_string = to_c_str(name);
        let value = unsafe { LLVMBuildAlloca(self.builder, ty.as_type_ref(), c_string.as_ptr()) };
        
        unsafe {
            LLVMSetAlignment(value, alignment);
        }

        unsafe { Ok(PointerValue::new(value)) }
    }

Alternatively, it could be done with an optional argument, but that would be less efficient:

    pub fn build_alloca<T: BasicType<'ctx>>(
        &self,
        ty: T,
        name: &str,
        alignment: Option<u32>
    ) -> Result<PointerValue<'ctx>, BuilderError> {
        if let Some(align) = alignment {
            // Check for valid alignment before building instruction.
            if !is_alignment_ok(align) {
                return Err(BuilderError::AlignmentError(
                    AlignmentError::NonPowerOfTwo(align)
                ));
            }
        }
        if self.positioned.get() != PositionState::Set {
            return Err(BuilderError::UnsetPosition);
        }
        let c_string = to_c_str(name);
        let value = unsafe { LLVMBuildAlloca(self.builder, ty.as_type_ref(), c_string.as_ptr()) };
        
        if let Some(align) = alignment {
            unsafe {
                LLVMSetAlignment(value, align);
            }
        }

        unsafe { Ok(PointerValue::new(value)) }
    }

In my opinion, the best solution is to have a separate method. It requires code duplication, but that could likely be solved with a macro if necessary.

Describe possible drawbacks to your solution
Option A (new aligned methods): Code duplication.
Option B (Option alignment param): More complicated code.

Describe alternatives you've considered
The alternative is to attempt to turn the result of these methods into an InstructionValue, which requires handling the result returned from the method, calling as_instruction, unwrapping that result, then setting the alignment on the instruction with another fallible function. The old method of setting the alignment could be a minor efficiency problem from all the branching required.

Edit: I forgot to mention, I'm willing to implement this change if I get approval. I'd just need feedback on preferences for how the functionality is implemented.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions