Page MenuHomePhabricator

[FPEnv] [WIP] Verify strictfp attribute correctness, first part, 2023 edition
Needs ReviewPublic

Authored by kpn on Mar 24 2023, 1:52 PM.

Details

Summary

This is a rewrite of D68233. Enough of the patch has been rewritten in the two years that I think it makes sense to just open a new review. I've copied over the reviewers and subscribers, and arsenm had a blocking request on D68233 that I've brought over here.

The strictfp attribute is now defined to require that function definitions be marked strictfp if they contain a strictfp call or a strictfp constrained intrinsic. This patch verifies that all function calls or called functions agree with their contained function about the strictfp attribute. It also enforces the attribute if a constrained fp intrinsic is used in a function.

Notably, the attribute is now allowed to be omitted from a call site if it is present on the declaration for the function being called. This is a change from the current Language Reference and the Langref should be changed in a subsequent patch if this change is accepted here.

This patch does _not_ require that all FP instructions in a function be strict if any of this is. That's a later patch.

Note that this patch _cannot_ yet go in the tree because quite a few tests break with this check in place. I'm working on fixing or getting those fixed, and having this patch visible is part of that.

Diff Detail

Unit TestsFailed

TimeTest
60 msx64 debian > Clang.CodeGen::fp-floatcontrol-class.cpp
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/17/include -nostdsysteminc -ffp-contract=on -triple x86_64-linux-gnu -emit-llvm -o - /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/fp-floatcontrol-class.cpp | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/fp-floatcontrol-class.cpp
80 msx64 debian > Clang.CodeGen::fp-floatcontrol-pragma.cpp
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/17/include -nostdsysteminc -fexperimental-strict-floating-point -DEXCEPT=1 -fcxx-exceptions -triple x86_64-linux-gnu -emit-llvm -o - /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/fp-floatcontrol-pragma.cpp | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck -check-prefix=CHECK-NS /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/fp-floatcontrol-pragma.cpp
80 msx64 debian > Clang.CodeGen::fp-floatcontrol-stack.cpp
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/17/include -nostdsysteminc -triple x86_64-linux-gnu -ffp-contract=on -DDEFAULT=1 -emit-llvm -o - /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/fp-floatcontrol-stack.cpp | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --check-prefix=CHECK-DDEFAULT /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/fp-floatcontrol-stack.cpp
70 msx64 debian > Clang.CodeGen::fp-template.cpp
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/17/include -nostdsysteminc -triple x86_64-linux-gnu -emit-llvm -o - /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/fp-template.cpp | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/fp-template.cpp
140 msx64 debian > Clang.CodeGenOpenCL::cl20-device-side-enqueue-attributes.cl
Script: -- : 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/17/include -nostdsysteminc -fno-ident -no-enable-noundef-analysis /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGenOpenCL/cl20-device-side-enqueue-attributes.cl -cl-std=CL2.0 -ffake-address-space-map -O0 -emit-llvm -o - -triple "spir-unknown-unknown" -fdenormal-fp-math-f32=preserve-sign -cl-uniform-work-group-size | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck --check-prefix=SPIR32 /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGenOpenCL/cl20-device-side-enqueue-attributes.cl
View Full Test Results (92 Failed)

Event Timeline

kpn created this revision.Mar 24 2023, 1:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 24 2023, 1:52 PM
kpn requested review of this revision.Mar 24 2023, 1:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 24 2023, 1:52 PM
arsenm added inline comments.Mar 24 2023, 4:33 PM
llvm/lib/IR/Verifier.cpp
3322–3325

Should go through the CallBase queries? getCalledFunction doesn't work for aliases and indirect calls

arsenm added inline comments.Mar 24 2023, 4:37 PM
llvm/test/Verifier/fp-intrinsics.ll
129

Test indirect call? and alias?

kpn updated this revision to Diff 511147.Apr 5 2023, 10:07 AM

Update for review comments.

kpn updated this revision to Diff 511786.Apr 7 2023, 1:37 PM

Verify that CallBase::isStrictFP() continues to do what we think it is doing.

arsenm added inline comments.Apr 7 2023, 3:14 PM
llvm/lib/IR/Verifier.cpp
6030

Drop "== true"

llvm/test/Verifier/fp-intrinsics.ll
54–58

Something is wrong here, there should be no error. The parent function has strictfp, and strictfp should always be implied by the intrinsic definition