In D76190#1948762, @MaskRay wrote:

@pengfei You can strip unneeded Phabricator tags with:

% which arcfilter
arcfilter () {
        git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: /,
"");print}' | git commit --amend -F -
}

Subscribers: and Reviewers: are just annoying. It is also not appropriate to use Patch By: Xiang Zhang (xiangzhangllvm). See https://llvm.org/docs/DeveloperPolicy.html#commit-messages " git commit --amend --author="John Doe <jdoe@llvm.org> to correct the author property"

Doesn't the modifier {z} clean the upper 64 bits?

LGTM.

Agreed, LGTM.

Address review comment.

Rebase and add vector instructions test.

In D72824#1835568, @RKSimon wrote:

Add to isNegatibleForFree/getNegatedExpression as well?

Remove unnecessary comment.

Address review comments.

Removed the FP_CONTRACT support code.

Add FMA4 tests. Thanks @RKSimon.

Address review comment.

Address review comments.

Remove dead code.

Yeah, it's useful and clear to use a STRICT_FP_EXTEND. Anyway, I think we can also achieve it by getting the function's attribute or propagating the fpexcept to FP_EXTEND.

Do we really need to distinguish STRICT_FP_EXTEND and FP_EXTEND for a return? Currently the STRICT nodes are used to avoid to be optimized out or reordered. But I think neither occurs for a return.

LGTM.

Add Fixme for softenSetCCOperands. We don't want to add extra logic code instead of pushing the libraries to do the right thing.
Address review comment.

After a deeper investigation, I agreed with Craig that we should "expand the unsupported calls into multiple compares and control flow in IR". But I think we don't need libcalls. We can get sNaN, qNaN and compare results according to control flow and IRs. We only need to define a target dependent interface TLI.raiseFPException to handle exception respectively.

LGTM.

In D71897#1796679, @craig.topper wrote:

There are libcalls for OGT/OLT/OLE/OGE that are always signalling. There is a libcall for OEQ/UNE that not signalling. And there's a libcall for Ordered/Unordered that is not signalling.

Is the signaling/no signaling action by design (the API guarantees the behavior) or depended on targets. I don't find the description about signaling on GCC document. I guess it depends on targets.

Note, that despite having multiple names there are really only 3 total libcalls. The names are aliases and the return value is used to check direction. The OGT/OLT/OLE/OGE libcall returns -1, 0, or 1. The other oeq/une and the ordered/unordered just return true or false.

In fact, there're only two. one is used for OGT/OLT/OLE/OGE/OEQ/ONE, one for ORD/UOR.
Why didn't we expand form these two functions directly? I think it can reduce the code complicity. And it can improve performance by reducing potential calls.

We may need to expand the unsupported calls into multiple compares and control flow in IR. For example for non-signalling ogt/olt/ole/oge we'll need to check unordered first and branch accordingly for qnans without calling the signalling ogt/olt/ole/oge libcall which will signal. Technically we could use a select to change the value we pass to the ogt/olt/ole/oge when its signalling, but select has to be expanded into control flow on many targets anyway. @uweigand, what are you thoughts on this?

I think it can be simplified to directly use __unordXf2 and __cmpXf2, if we know it's a NaN from the former, we can call a new function TLI.raiseNaNException to handle exception respectively.

Do you have plan for uint_to _fp and signed i64? Otherwise, I can do it.

Minor fixup.

Address review comments.

In D71862#1795767, @rnk wrote:

In D71862#1795766, @pengfei wrote:

In D71862#1795756, @rnk wrote:

lgtm

Does funclet not need rbp to access parent local variables now?

No, it does need to go via ebp/rbp, but this attribute appears to no longer be required to achieve that.

In D71862#1795756, @rnk wrote:

lgtm

Thanks. LGTM.

Address review comments.

Address review comments.

Ping.

Address review comments.

Removed unnecessary loop condition.
Added a vectorized index test.
Don't check if constant exist in DAG.

Removed unnecessary else.

Removed unnecessary assertions. Merged some common code.

Optimized as Yuanke's suggestion.

It's good that people are looking at achieving better modeling for the x86 backend, but we need to have a plan that doesn't require heroic effort just to get basic correctness.

Do you mean in the backend? If so, I don't think that's possible. The backends just don't have any sort of feature that could be used to get conservatively correct behavior for cheap the way intrinsics give it to us in the middle end. Once you go into instruction selection things get very low level in a hurry.

I'm looking for simple ways to modeling X86 intrinsics, but haven't find better one than modeling it one by one.

I would suggest that we need a function/call attribute roughly on the level of readonly / readnone, maybe readfponly, that says that a function has no side-effects and no dependencies on anything *except* the FP state.

Do you mean mark it at the declaration of intrinsics? Is it reasonable to mark except on dependent intrinsics?

Basic queries like Instruction::mayReadMemory() that are supposed to be used generically in code-motion transforms would then return true for calls marked that way only if they're FP-constrained functions.

Middle end or back end? I think in middle end you may need to change all releated passes to get such information to prevent optimization. And in back end, I think we can simply chain intrinsics marked except with other FP nodes like what common code doing.

We don't need to check if constant is defined.

Using gep_type_iterator to traverse.

Updated to only check the final type.