[PATCH] D44909: [DAGCombine] (float)((int) f) --> ftrunc (PR36617)

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 11 19:12:05 PDT 2018


spatel added a comment.

In https://reviews.llvm.org/D44909#1065089, @thakis wrote:

> This broke some large integration test on arm64 in Chromium (https://crbug.com/831145 – doesn't yet have any information other than "something is broken" and that we bisected it to this revision). It sounds like the patch got rewritten during review; does "formally allowed by the standard, but extremely likely to break things and surprise people" still apply? It sounds like this is supposed to be behavior-preserving for values that aren't NaN and Inf, but it does change behavior for those two? (If so, in what way?)


The patch started off as an IR transform, and the quoted comment applied only to cases (I hope) where the target doesn’t have native support for an fptrunc instruction. NaN/inf inputs are undefined with these ops, so if something was relying on that undefined output, that could be the source of the problem.

I’m not an arm64 expert, but the tests show that we should codegen ‘frintz’ instructions with this patch. So that would be the place to look for diffs.


Repository:
  rL LLVM

https://reviews.llvm.org/D44909





More information about the llvm-commits mailing list