amy-kwan wrote: > > Uhm - this looks pretty big and seems like something that can easily break > > certain build configurations since it doesn't seem to touch only AIX > > Agreed that this looks big and scary, but it's a purely mechanical change, > that is a no-op for most targets. I'll add a long form rational at the end of > the comment about why I don't think the patch effects anyone but AIX to keep > my answers brief. > > > Is this in main without any issues? > > Yes, these patches have been in main for several weeks at this point with no > reported issues. > > > Does it really NEED to be merged for the release branch at this point? > > It would help us out for the point releases. Without this patch, we're unable > to build on AIX with CMake from our package manager (4.0). We can manually > downgrade if we're unwilling to merge this, but it's a bit of a pain. > > **Rationale about why the patch doesn't affect targets besides AIX** > > We quote the string AIX and variable expansions which might expand to string > AIX (i.e. `CMAKE_SYSTEM_NAME`), so that we do the intent string comparison. > If not quoted the if will expand the string if it happens to match a variable > name (which `AIX` does in CMake 4.0+). > > This has an effect only if `CMAKE_SYSTEM_NAME` > (https://cmake.org/cmake/help/latest/variable/CMAKE_SYSTEM_NAME.html) expands > to something which is a CMake variable > (https://cmake.org/cmake/help/latest/manual/cmake-variables.7.html#variables-that-describe-the-system) > > Intersecting the two list gives me the following list of affect targets: > > ``` > AIX > CYGWIN > MSYS > WASI > ``` > > Of those targets, only CYGWIN appears in the lines affected by the patch, and > it's already using a variable check (i.e. it checks `CYGWIN`) not a string > comparison to `CMAKE_SYSTEM_NAME`, so it's unaffected.
Hi @tru! Any thoughts regarding @daltenty's response on this backport? https://github.com/llvm/llvm-project/pull/156505 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
