[SandboxIR] Adding isVolatile arg to existing LoadInst::create() functions

[wizardengineer]

wasn't too sure if i was suppose to add the body, just added the function header for now

cc: @vporpo

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[wizardengineer]

@vporpo

#100717 (comment) to follow up this comment, I'll make sure to add a body and a test for this new function.

[vporpo]

I would move the isVolatile flag before the Ctx argument.
Also we would need both versions of the create() function with the additional isVolatile argument + their bodies + tests :)

[wizardengineer]

I would move the isVolatile flag before the Ctx argument.

will do

Also we would need both versions of the create() function with the additional isVolatile argument + their bodies + tests :)

so for every create function there needs to have a version where isVolatile is an argument?

[vporpo]

yes, 4 in total: 2 with isVolatile and 2 without.

[wizardengineer]

I think we can save some lines of code, do you think it'd be more sensible to just have 2 functions instead of 4. Where the isVolatile arg, has default value of false if there's no input? or should we stick to the original plan of having 4 functions?

[vporpo]

Makes sense. Something like create(Type *Ty, Value *Ptr, MaybeAlign Align, Instruction *InsertBefore, Context &Ctx, bool IsVolatile = false, const Twine &Name = "") ? I was suggesting using 4 functions because llvm::LoadInst is using separate constructors but I don't feel too strongly about it.

[wizardengineer]

do you approve? if so, i'll have to change the title to something more suited for our situation

[vporpo]

Yes, go for it.

[wizardengineer]

It's a pain to deal with, I'll stick with the former suggestion. After trying to deal with it, I realized it creates ambiguity in the code

[vporpo]

Either way is fine with me.

[wizardengineer]

@vporpo I'll have to close this PR unfortunately and ship the code to another PR. My local branch got corrupted and will create a mess with this PR if I tried to do a commit.