User Details
- User Since
- Oct 21 2013, 6:34 AM (519 w, 2 d)
Apr 4 2023
Apr 3 2023
Jul 6 2022
Jul 1 2022
run tests once
Jun 27 2022
May 3 2022
bump? this should be an easy obvious bugfix
I would be supportive of a patch that added a TTI function that returned the contents of the target-attribute when it is present or the TargetMachine attributes when it is not.
Apr 14 2022
Apr 13 2022
I noticed the first 2 RUN statements in the file were accidentally identical, so I fixed one and reran the update_llc_test_checks.py to fix the tests
Apr 12 2022
I was tracking back a recent ABI break (also failing now in gcc 12, so maybe this irregularity is intentional), and was concerned that this commit is observed to cause the platform ABI to change depending on the feature flags of the current compilation unit. Prior to this change, f16 was always treated as i16 for the purpose of the calling-convention (e.g. returned in %ax). But after this change, the ABI of the value is now inconsistent between compile units. I made a small change to one of the existing tests to show this. Note how the callq result was in %ax without this mattr flag, and in %xmm0 with this mattr flag added. But the function known as "identity.half" is external, and did not change between those two calls to the llvm.
Mar 8 2022
bump?
bump?
Feb 28 2022
Hm, I am actually not 100% certain, since I think that might be a current bug now that I think about it, which was caused by failing to expose this information about the TargetMachine. It looks some passes are adding flags there (e.g. +thumb/-thumb) with the implied assumption that they should be inheriting the global TargetMachine flags for anything left unspecified, while other passes (e.g. msan) are checking only the Function's flags, with the implied assumption that the configured TargetMachine does not specify target-features, and yet other passes (MIR/Codegen) are probably assuming that the "target-features" flag should entirely replace and override the TargetMachine's feature set. These assumptions seem currently incompatible.
It can matter in what ways the the "target-attribute" is different. Just grepping the source tree reveals there are several existing IR passes that depend on examining this function attribute, and are already altering their behavior based on the value. For example, it appears that the msan pass should be falling back to examining the TTI's notion of the target machine to locate the function arguments correctly, if I am reading the source comments here correctly:
TargetMachine is nearly always a required argument here, so clearly the analysis already does depend on the the TargetMachine. Sometimes the information we (aka JuliaLang) need is available in the "target-features" function attribute (which is always available), but we want to compare that against the TargetMachine's notion of the TargetFeatures during optimization. Those together are then nearly the complete set of arguments to the TargetMachine, so it seemed more logical to me to expose the TM directly, rather than separately exposing all of the information needed to reconstruct a copy of it. I don't think we should obfuscate this just for the sake of obfuscation, do you?
Feb 25 2022
ping @fhahn
Seems extra complexity, and less desirable to lose test coverage, when it seems the option may already be unreliable and soon to be deprecated by https://reviews.llvm.org/D119383. @MaskRay is that accurate?
fix
this is adding a new restriction
Feb 24 2022
Feb 22 2022
Is that a bug in clang or in MSVC2019? We expected that CLANG_PLUGIN_SUPPORT=ON can expose problems with the Windows linker, and we add new tests here for verifying that configuration is functioning as expected. You might need to disable CLANG_PLUGIN_SUPPORT on that target?
Feb 17 2022
Feb 16 2022
Okay, this should work now for those cases! Sorry, I kept thinking it was clang/examples that was broken, and missed that it was clang/test that was failing, so I failed to see what I needed to fix there too, so that we were not trying to build the tests in configurations where the functionality is disabled.
add one more missing case to check
Feb 14 2022
fixup
We are not going to start exporting plugin support if the user explicitly disabled it with CLANG_PLUGIN_SUPPORT=OFF, but why is it trying to read that directory at all, when it should be disallowed by this PR unless CLANG_PLUGIN_SUPPORT=ON
cleanup
Feb 11 2022
Feb 9 2022
Thanks, if you encounter more issues next week, let me know, and I will continue trying to adjust it to work for all config option combinations.
@cristian.adam Is this good now? You are blocking merging this, but I think it is ready to land, and I would like to not hold it up for other people if it is fixing their issues (and the issues you discovered too)
Feb 8 2022
ensure CLANG_PLUGIN_SUPPORT setting is compatible with llvm_add_library
Feb 7 2022
Fixed by D119199
Ah, this looks annoying: there are apparently two flags, CLANG_PLUGIN_SUPPORT and LLVM_ENABLE_PLUGINS, but the existing build for clang uses CLANG_PLUGIN_SUPPORT to turn off the build support and LLVM_ENABLE_PLUGINS to turn off the tests (you might not have noticed this existing issue since you turned off CLANG_ENABLE_STATIC_ANALYZER support, and CLANG_BUILD_EXAMPLES is off by default, which looks like it would disable all of the existing tests for this functionality). This might fix this particular test?
diff --git a/clang-tools-extra/test/CMakeLists.txt b/clang-tools-extra/test/CMakeLists.txt index 9321457ae1a3..c98ec90a179b 100644 --- a/clang-tools-extra/test/CMakeLists.txt +++ b/clang-tools-extra/test/CMakeLists.txt @@ -17,7 +17,7 @@ string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} CLANG_TOOLS_DIR ${LLVM_RUN
It is a somewhat worthless test IMO, and might belong better in LLVM itself (where this functionality is defined), but there does not appear to be any other like it currently, and it was requested by a previous reviewer. Comparing to the code in LLVMTestingSupport, does this fix it for you:
diff --git a/clang-tools-extra/test/CMakeLists.txt b/clang-tools-extra/test/CMakeLists.txt index 9321457ae1a3..17cc12473565 100644 --- a/clang-tools-extra/test/CMakeLists.txt +++ b/clang-tools-extra/test/CMakeLists.txt @@ -87,6 +87,15 @@ if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY) PLUGIN_TOOL clang-tidy DEPENDS clang-tidy-headers)
Feb 2 2022
TargetTransformInfo does not currently have what I need, but if I can add it there, it would also satisfy my needs. I wanted it for an optimization pass (to pre-populate some codegen decisions earlier in the pipeline for better performance), but it looks like all current in-tree consumers are codegen-related.
Feb 1 2022
rebase
@fhahn You added these test failures recently, but they are working now. Does this look good to you to enable them?
rebase on main branch
I don't know the plan specifics, but I am just exposing an existing analysis pass to the command line that already has been converted, not changing it to define any new passes.
I decided it made the most sense to me to go with option 3, so this should be ready to land again.
- Reland "enable plugins for clang-tidy"
- fixup! Reland "enable plugins for clang-tidy": Disable the test if the user has disabled support for building it.
Jan 31 2022
It looks like this is probably the first time that a test was written for a PLUGIN_TOOL, outside of the docs, so we are likely in new territory here :/
Jan 30 2022
Yes, please push a revert so I can look later. Do you have a link to the buildbot configuration, so I can reproduce that?
Jan 29 2022
Jan 26 2022
@aaron.ballman I think I incorporated all of your feedback. Is this okay for me to merge to main? I would like to get it in before the feature branch.
Jan 21 2022
arc error
address review feedback
Jan 7 2022
Can you upload the full diff (git diff -U100000 or arc diff --update D116852) and fix the clang-format check?
Dec 16 2021
Thanks! I will work on making those changes
Dec 8 2021
remove accidental commit added by arc
run update_test_checks.py on these files too
The work had already pretty much been done before (for GVN), we just needed to
allow it to use that work here for NewGVN, and use bits directly rather than
bytes/8*8 for the output computation.
rebase onto LLVM main, allowing it to handle null values everywhere, and not just for non-integral pointers
fix cmake
Nov 30 2021
There is clearly some more work to do to get the cmake file to be correct, but was hoping to check this looked like the direction you thought looked right for adding this test, since there isn't an obvious example to follow.