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

[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.

[TheDan64]

Might be simpler for PointerValue to have a set_alignment method no? Possibly on the BasicValue trait if it applies to all basic values

[ErisianArchitect]

You can't set the alignment for any PointerValue, only for InstructionValues of a subset of types (store, load, alloca, cmpxchg, and maybe some more).

Hence why you need to convert to an InstructionValue in order to set the alignment. And that would still require some overhead with unwrapping the result and then converting to an InstructionValue, unwrapping that result, then calling set_alignment, which has to do additional checks.

    pub fn set_alignment(self, alignment: u32) -> Result<(), InstructionValueError> {
        // Zero check is unnecessary as 0 is not a power of two.
        if !alignment.is_power_of_two() {
            return Err(InstructionValueError::AlignmentError(AlignmentError::NonPowerOfTwo(
                alignment,
            )));
        }
        if !self.is_a_alloca_inst() && !self.is_a_load_inst() && !self.is_a_store_inst() {
            return Err(InstructionValueError::AlignmentError(
                AlignmentError::UnalignedInstruction,
            ));
        }
        unsafe { LLVMSetAlignment(self.as_value_ref(), alignment) };
        Ok(())
    }

LLVMSetAlignment is safe to use on the above subset of InstructionValues so long as the alignment is valid, which can be checked with an is_power_of_two predicate, which is cheaper than having to check alignment as well as check instruction type.

This API change would simplify the whole process and make code easier to read, although it might require code duplication in some way.