Data-Oblivious scf.if Transformation

[MeronZerihun]

Draft of an implementation for a pass that hoists instructions out of an scf.if operation and finally substitutes it with a select-operation.

[AlexanderViand-Intel]

👍 Let's work on getting the tests rewritten to FileCheck today and then we can focus on the required data analyses next :)

[AlexanderViand-Intel]

Thanks for putting this together! Despite my many (mostly nit-picky) comments, I think this actually looks great, especially the IfToSelectConversion Pattern is significantly more elegant/simple than I would have initially expected possible!

[j2kun]

In lieu of a proper greeting during the meeting this morning, Hello @MeronZerihun! It's great to have you in our merry band of cryptographers. This PR looks very well done and thorough so far, and I'll give it a second pass after you address Alex's review comments.

A dataflow analysis is the right approach, but in the interim you can get away with something as simple as this, which checks if the condition to the if is a direct operand of the secret.generic op containing the if:

if (auto genericOp = ifOp.getParentOfType<secret::GenericOp>()) {
  if (OpOperand operand = genericOp.getOpOperandForBlockArgument(condition)) {
    // it's an OpOperand to the containing secret.generic
    if (isa<secret::SecretType>(operand.get().getType())) {
      // it's a secret!
    }
  }
}

[j2kun]

LGTM! One optional change suggested, then let's get it merged.

[AlexanderViand-Intel]

LGTM, too, just some minor cleanup around the tests and then we should be able to get this merged👍

[AlexanderViand-Intel]

@j2kun / @asraa Any idea why the other actions aren't showing up after the last push?

[j2kun]

That is bizarre... maybe GitHub had a brain fart. Try force-pushing again with a trivial change?

[asraa]

Hey! @MeronZerihun super big apologies, but could you rebase cd909c8 on top of main? (Sorry for attempting to resolve the commit with my last merge commit, I had a mental fart too and realized we needed to squash commits)

[AlexanderViand-Intel]

Oh, wow, this PR is really hitting all the CI issues xD Now we've got a Google internal checks FAILED?
@asraa Is it something we need to fix on our side, or something you can fix internally?

[j2kun]

The internal checks failing is normal. It just means we have to click a few extra buttons to get it to pass internal CI, which is usually build rule fixes that only apply internally.

…

On Tue, Jul 2, 2024, 1:54 PM Alexander Viand ***@***.***> wrote: Oh, wow, this PR is really hitting all the CI issues xD Now we've got a Google internal checks FAILED? @asraa <https://github.com/asraa> Is it something we need to fix on our side, or something you can fix internally? — Reply to this email directly, view it on GitHub <#737 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAS2PKX5KDKNLYA5IS3ST2TZKMHQNAVCNFSM6AAAAABJK5DQ22VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMBUGM4TKMJXG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[asraa]

Oh, wow, this PR is really hitting all the CI issues xD Now we've got a Google internal checks FAILED?

Ah yes, all good now! I can now take it from here to fix up the build issues and merge :)