[PATCH] D31946: [legalize-types] Make softening result use a single map for replacements.

[Anton Yartsev via Phabricator via llvm-commits]

ayartsev added a comment.

> Does the new patch work with my previous attached c.zip?
>  I don't see new changes related to CopyToReg.

I've managed to reproduce the error from your attached c.zip. Here is the minimal test derived from the //fdiml()// implementation (the origin of the error as seen from the error message):

  long double fn(long double x, long double y) {
    return (x > y ? x - y : 0.0);
  }

The problem was not related to CopyToReg but to handling of ISD::SELECT. Before this patch operands of ISD::CopyToReg, ISD::SELECT, etc. were implicitly replaced by ReplaceSoftenFloatResult() called from SoftenFloatResult(). Now operands are explicitly replaced by their softened versions (if any) in SoftenFloatOperand().

> Again I worry about the new changes to FABS, FCOPYSIGN, and SELECT.
>  Why do you need those changes, but not FNEG, Register, etc.?
>  Those op codes used to cause problems with Android x86_64 fp128 types.
>  That's why I have them and some other opcodes in the
>  CanSkipSoftenFloatOperand function.

Operands of CopyToReg, FABS, FCOPYSIGN, and SELECT may have been softened and must be replaced by SoftenFloatOperand() as I mentioned above. As for other opcodes - FNEG, Register, etc. - I'm currently failing to write regression tests for this opcodes. It may be impossible to have softened operands for this opcodes.

> Does the new test soft-fp-legal-in-HW-reg.ll catch any error?
>  It should have some CHECK tags to verify compiled output.

The new test checks that stderr is empty and  we don't end up with //"fatal error: Cannot select:..."//. Similar tests exist among LLVM tests, look at ##\test\CodeGen\X86\xmm-r64.ll## for example.

> It should also be tested with the android x86_64 mode

Tested with //-mtriple=x86_64-linux-android//. The result remains the same. The patch cause no regression in LLVM tests. Do you have a chance to test the new patch over Android codebase?

Thanks for the feedback!


https://reviews.llvm.org/D31946