For the next step, what can I do?

• Rebase
• Remove AsmPrinter::emitFreeze

Just noticed that I landed this without explicit accept; sorry.
PR44543 is now resolved.

https://reviews.llvm.org/rG0877843ddacc applied, update diff from master

Address comments

Minor update

BTW, there was a discussion about IMPLICIT_DEF as well, about whether it should return consistent value for every use vs. it can return different values per use (as IR's undef).

To synchronize - the remaining issue was about duplicability of the freeze instruction in MIR.

• Rebase
• Address a comment

• Address a comment

ping

LGTM!

Sorry for my delay.
I updated the function name & added a unit test.

I added description about FREEZE and IMPLICIT_DEF to TargetOpcodes.def.
To address the legalization issue, updated the semantics so FREEZEs return the same value if placed consecutively. Would this properly address the issue?

Currently ScalarEvolution::getBackedgeTakenCount() is commented as follows, which is not clear about the case when instruction returning nondeterministic value is involved (such as freeze).

Fixed by https://github.com/llvm/llvm-project/commit/1465b8bc3a2435eab46582616bdf7c6aee117e8d

Hi, sorry I'll fix it soon, perhaps in an hour or so.

In D70246#1749243, @liuz wrote:

Hi Juneyoung. Is that a typo? undef & 0 should be 0.

This patch looks good to me. @nlopes @regehr @lebedev.ri

In D70246#1747526, @spatel wrote:

This is correct based on my understanding of poison (and what we've implemented so far), but we do not explicitly state that poison can occur per-element of a vector. Should we add a LangRef edit here or in a separate patch to address that?

• Add nounwind to tests

In D70246#1746403, @spatel wrote:

AFAIK, no - but that's because we were using 'undef' in the docs here:
http://llvm.org/docs/LangRef.html#shufflevector-instruction
(no mention of poison)

The optimization can be kept live by adding freeze (shufflevector <4 x float> %arg, ... -> freeze %arg), but existing analyzers are not aware of freeze yet (which will block following optimizations) and the underlying problem is more related with interaction of undef/poison (which disappears if undef is removed), so I'm fine with this patch's direction.

• Add freeze IR -> MIR test

• Let FastISel emit FREEZE
• Let SelectionDAGBuilder::visitFreeze lower freeze IR instructions
• Minor fixes

In D29013#1723781, @reames wrote:

LGTM.

p.s. Do you have a list of items for this work somewhere? We clearly need support in a number of passes, is there a master list so that we can split work once the IR def pass lands?

I added FREEZE pseudoinstruction to MachineIR, as it seemed to be the succinct way to make it correct.
I made ExpandPostRAPseudos expand the FREEZE to register-copy assembly instruction. ExpandPostRAPseudos pass is done after register allocation (which, or at least a pass relevant to it, seems to replace uses of IMPLICIT_DEF with different registers) as well as ProcessImplicitDef (which is the culprit of the incorrect PHI optimization example) and PHIElimination. I left a commit that explicitly states that IMPLICIT_DEF cannot be used in an undef-y way after the pass.

• Add FREEZE pseudoinstruction for MachineIR
• Let ExpandPostRAPseudos expand FREEZE
• Address reviewers' comments
• Add tests for function call / phi
• Add a comment to TargetPassConfig stating that after ExpandPostRAPseudos IMPLICIT_DEF should not be optimized to different registers/constants

The new patch is here: https://reviews.llvm.org/D69932

Previous patch: https://reviews.llvm.org/D29011

In D29011#1735007, @craig.topper wrote:

Does a ConstantExpr freeze really make sense? ConstantExprs are uniqued by the LLVMContext. So every constantexpr that has the same input ends up being the same object. Do we want that behavior do we need each freeze to be distinct?

In D29011#1733572, @craig.topper wrote:

Sorry to come to this late, but does it really make sense for Freeze to be inherited from UnaryOperator? UnaryOperator in my mind is like BinaryOperator which is for arithmetic operations. Freeze is different. The fact that it made sense to add a FreezeOperator hints at that. Should we instead just have a FreezeInst separate from UnaryOperator instead of FreezeOperator?

I was looking through MachineInstr's optimizations to recheck validity of using IMPLICIT_DEF, and found that it caused problems at two cases.

• Remove DAGTypeLegalizer::PromoteIntOp_FREEZE, DAGTypeLegalizer::ExpandIntOp_FREEZE as they are never called
• Add cost model of freeze
• Add vector legalization test to freeze-legalize.ll

Sorry, I've been busy recently.

I have no permission to land this patch - what is the best way for the next step?

I agree that the size of a block should not be larger than the half of memory size.

• Support floating points, pointers, aggregate types, add tests for those types

• Use FreezeOperator inside Freeze_match

• Rebase
• Add more tests to freeze.ll
• Define FreezeOperator

In D29011#1707278, @lebedev.ri wrote:

Should you add llvm::Freeze here by inheriting from UnaryOperator to make isa<Freeze>(Op) possible?

• Support freeze constexpr
• Make freeze's type checking consistent with LangRef.

should there be a constantexpr freeze

Updated the patch so UnaryOperator is used for Instruction::Freeze op.
Currently I see an error from Phabricator while uploading a new diff:

Unhandled Exception ("PhabricatorFileUploadException")	
Unable to write file: failed to write to temporary directory.

I'll retry uploading it later.

Rebase to trunk@369887 (August 26th, 2019)

Rebase to trunk@369887 (August 26th, 2019)

Rebase to trunk@369887 (August 26th, 2019)

Sorry, I was pretty busy last week.
@xbolva00 Rebased

One mode of reasoning that could invalidate this example is that p *could have been* something other than 2^64-4 which would invoke UB. This UB would both let us pretend that foo does not escape p, and also print 2^64-4 even though p was not 2^64-4 in the execution we chose. This seems a bit subtle though, and I'm not entirely sure this is sound reasoning.

In the example, the original pointer (%addr = load %struct.zot*, %struct.zot** %loc, align 8) is not dereference directly and the use case I am looking at is tagged pointers, where the inttoptr(and(ptrtoint(), C) roundtrip is required to get a valid pointer. So the original pointer might not be dereferenceable directly, but logically (ignoring the bits irrelevant for the pointer value) it should still point to the same object. Does that make sense to you?