Instcombine will convert the nonnull and alignment assumption that use the boolean condtion
to an assumption that uses the operand bundles when knowledge retention is enabled.
Please add some documentation here.
I don't get this. Doesn't it replace the operand of an assume with false if there is one with the same condition following? That seems wrong, maybe "true" is what you wanted?
Comments on what is happening here and below please.
Isn't this call the same as the 5 lines before?
It is odd that this takes an insertion point but apparently does not insert the instruction. Can we change either to confuse me less?
Why do we have SAME checking the function twice? Is this a bug in the update script?
Looks conceptually good but there are some oddities, potentially in the lookup.
Can u add a FIXME or NOTE here to indicate we could duplicate the load instead and keep a assume with ["nonnull"(%load)] around.
Again a fold missing CMP = true
I had to merge the previous patch with other i had laying around and add some more
to make the bundle mode be at least as good as the default. so this revision got
quite a bit bigger.
I hope its still fine.
This is not a strict order. Let's use AttrKind and WasOn as well.
Add a sentence what this controls/means.
I'd add a message to the assert and a comment before the lambda.
Shouldn't we build a GEP for the offset instead, if the offset is not a multiple of the alignment that is.
Nit: Add a message to the assert please explaining what usage is allowed.
Nit: Add a comment describe what is happening here.
I guess we should have a todo that states partial information, e.g., only AlignRK or DerefRK, should be used in other places too. The entire function should be rewritten for that so it is out of scope.
At least a TODO would be good.
I can do the upstreaming but i don't know what the current state is for upstreaming this.
this is one of the default canonicalization done when building assumes maybe we should keep both.
I think there are different things in this patch, some can be upstreamed some need minor adjustment, and some I don't understand yet. See my comments. It would be great if we can finish this line of assume optimizations though.
Nit: The first sentence is hard to read.
Why can we allow ephemeral here without potentially throwing away an assume that was used to argue it itself is useless? Should we not do the AllowEphemeral stuff for now?
Nit: Typo, arguement
Nit: typo: help
I'm not sure why we return nullptr here and not continue going.
!Assume means Assume->GetModule() will crash, probably add a DL argument.
How do we get an alignment of 8 here?