Page MenuHomePhabricator

[PatchableFunction] Allow empty entry MachineBasicBlock
ClosedPublic

Authored by MaskRay on Jan 23 2020, 3:32 PM.

Diff Detail

Event Timeline

MaskRay created this revision.Jan 23 2020, 3:32 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 23 2020, 3:32 PM

(sorry, crash is different w/ and w/o --target=aarch64-linux-gnu)

Also, I just used llvm-reduce for the first time to get:

; ModuleID = 'quarantine.ll'
source_filename = "quarantine.i"
target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
target triple = "aarch64-unknown-linux-gnu"
; Function Attrs: nounwind
define dso_local i32 @a() #0 {
entry:
  ret void
}

attributes #0 = { nounwind "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="non-leaf" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "patchable-function-entry"="0" "stack-protector-buffer-size"="8" "target-cpu"="generic" "target-features"="+neon" "unsafe-fp-math"="false" "use-soft-float"="false" }

!llvm.module.flags = !{!0}

!0 = !{i32 1, !"wchar_size", i32 4}
MaskRay updated this revision to Diff 240036.Jan 23 2020, 3:48 PM

Add MIR test

MaskRay updated this revision to Diff 240043.Jan 23 2020, 4:12 PM

Improve test

Unit tests: fail. 62146 tests passed, 5 failed and 811 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

Unit tests: fail. 62147 tests passed, 5 failed and 811 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

Unit tests: fail. 62146 tests passed, 6 failed and 811 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp
failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_timedmutex_requirements/thread_timedmutex_recursive/try_lock_until.pass.cpp

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

MaskRay edited the summary of this revision. (Show Details)Jan 23 2020, 5:19 PM
nickdesaulniers accepted this revision.Jan 24 2020, 9:38 AM

Thanks, with this patch applied, I was able to build an allyesconfig aarch64 linux-next kernel, and the llvm test suite passes. LGTM

This revision is now accepted and ready to land.Jan 24 2020, 9:38 AM
MaskRay updated this revision to Diff 240235.Jan 24 2020, 9:44 AM

Simplify test

This revision was automatically updated to reflect the committed changes.

Thanks, with this patch applied, I was able to build an allyesconfig aarch64 linux-next kernel, and the llvm test suite passes. LGTM

What is the linux-next kernel?

I tested with (it needed a -DLLVM_ENABLE_ASSERTIONS=on build to trigger an assertion failure in ilist_iterator.h):

make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- HOSTGCC=gcc CC=~/llvm/ReleaseAssert/bin/clang LD=~/llvm/ReleaseAssert/bin/ld.lld O=/tmp/arm64 allmodconfig all -j 30

MaskRay added a subscriber: hans.Jan 24 2020, 9:54 AM

@hans OK to cherry pick this commit? We would also need d232c215669cb57f5eb4ead40a4a336220dbc429 ([AsmPrinter] Don't emit __patchable_function_entries entry if "patchable-function-entry"="0"; a GCC compatibility issue I did not notice when I implemented -fpatchable-function-entry=N,0).

If users want to see a complete -fpatchable-function-entry=N,M, instead of just -fpatchable-function-entry=N,0 , in clang 10.0.0, we will need D73070, D73071, and D73072 as well (N,M where M>0 is currently not used, though someone told me that Linux arch/arm64 may want to do that in the future. If that is the case, supporting a complete N,M, not just N,0 can simplify Linux kernel side configure test.)

Unit tests: fail. 62175 tests passed, 5 failed and 815 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

hans added a comment.Jan 24 2020, 10:18 AM

@hans OK to cherry pick this commit? We would also need d232c215669cb57f5eb4ead40a4a336220dbc429 ([AsmPrinter] Don't emit __patchable_function_entries entry if "patchable-function-entry"="0"; a GCC compatibility issue I did not notice when I implemented -fpatchable-function-entry=N,0).

Yes, go ahead.

If users want to see a complete -fpatchable-function-entry=N,M, instead of just -fpatchable-function-entry=N,0 , in clang 10.0.0, we will need D73070, D73071, and D73072 as well (N,M where M>0 is currently not used, though someone told me that Linux arch/arm64 may want to do that in the future. If that is the case, supporting a complete N,M, not just N,0 can simplify Linux kernel side configure test.)

I think you're the best judge of whether we should merge these. I'm okay either way.