[PATCH] D107452: [AArch64] Fix assert AArch64TargetLowering::ReplaceNodeResults

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Sounds OK to me, from a StrictFP perspective. I just have some test cleanup suggestions/nitpicks.



================
Comment at: llvm/test/CodeGen/AArch64/fptosi-strictfp.ll:1
+; RUN: llc < %s | FileCheck %s
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
----------------
It is commit in llc test to use -mtriple=aarch64-arm-none-eabi as opposed to the datalayout and triple below. But doesn't make a big difference either way.


================
Comment at: llvm/test/CodeGen/AArch64/fptosi-strictfp.ll:5
+
+define hidden void @test_fixtfti(fp128 %ld) local_unnamed_addr #0 {
+entry:
----------------
I think you can remove "hidden", and usually "local_unnamed_addr #0"


================
Comment at: llvm/test/CodeGen/AArch64/fptosi-strictfp.ll:8
+  %conv = call i128 @llvm.experimental.constrained.fptosi.i128.f128(fp128 %ld, metadata !"fpexcept.strict") #0
+  ret void
+}
----------------
Can we return "conv"? Otherwise something might decide to remove the instruction.


================
Comment at: llvm/test/CodeGen/AArch64/fptosi-strictfp.ll:10
+}
+; CHECK: bl	__fixtfti
+
----------------
Add CHECK-LABEL, and maybe move before the test_fixtfti function? Or just use the update script.


================
Comment at: llvm/test/CodeGen/AArch64/fptosi-strictfp.ll:23-24
+
+attributes #0 = { strictfp }
+attributes #1 = { inaccessiblememonly mustprogress nofree nosync nounwind willreturn }
----------------
Can probably remove these.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107452/new/

https://reviews.llvm.org/D107452