On 6/28/2021 1:16 AM, CHIGOT, CLEMENT wrote:
On 6/23/2021 12:53 AM, CHIGOT, CLEMENT via Gcc-patches wrote:
Hi David,
Did you have a chance to take look at this patch ?
Thanks,
Clément
+DavidMalcolm
Can you review this patch when you have a moment?
Thanks, David
On Mon, May 17, 2021 at 3:05 PM David Edelsohn <dje....@gmail.com> wrote:
The aix.h change is okay with me, but you need to get approval for the
incpath.c and cpplib.h parts of the patch from the appropriate
maintainers.
Thanks, David
On Mon, May 17, 2021 at 7:44 AM CHIGOT, CLEMENT <clement.chi...@atos.net> wrote:
On AIX, stat will store inodes in 32bit even when using LARGE_FILES.
If the inode is larger, it will return -1 in st_ino.
Thus, in incpath.c when comparing include directories, if several
of them have 64bit inodes, they will be considered as duplicated.
gcc/ChangeLog:
2021-05-06 Clément Chigot <clement.chi...@atos.net>
* configure.ac: Check sizeof ino_t and dev_t.
* config.in: Regenerate.
* configure: Regenerate.
* config/rs6000/aix.h (HOST_STAT_FOR_64BIT_INODES): New define.
* incpath.c (HOST_STAT_FOR_64BIT_INODES): New define.
(remove_duplicates): Use it.
libcpp/ChangeLog:
2021-05-06 Clément Chigot <clement.chi...@atos.net>
* configure.ac: Check sizeof ino_t and dev_t.
* config.in: Regenerate.
* configure: Regenerate.
* include/cpplib.h (INO_T_CPP): Change for AIX.
(DEV_T_CPP): New macro.
(struct cpp_dir): Use it.
So my worry here is this is really a host property -- ie, this is
behavior of where GCC runs, not the target for which GCC is generating code.
That implies that the change in aix.h is wrong. aix.h is for the
target, not the host -- you don't want to define something like
HOST_STAT_FOR_64BIT_INODES there.
You'd want to be triggering this behavior via a host fragment, x-aix, or
better yet via an autoconf test.
Indeed, would this version be better ? I'm not sure about the configure test.
But as we are retrieving the size of dev_t and ino_t just above, I'm assuming
that the one being used in stat directly. At least, that's the case on AIX, and
this test is only made for AIX.
It's a clear improvement. It's still checking for the aix target though:
+# Select the right stat being able to handle 64bit inodes, if needed.
+if test "$enable_largefile" != no; then
+ case "$target" in
+ *-*-aix*)
+ if test "$ac_cv_sizeof_ino_t" == "4" -a "$ac_cv_sizeof_dev_t" ==
4; then
+
+$as_echo "#define HOST_STAT_FOR_64BIT_INODES stat64x" >>confdefs.h
+
+ fi;;
+ esac
+fi
Again, we're dealing with a host property. You might be able to just
change $target above to $host. Hmm, that makes me wonder about canadian
crosses where host != build. We may need to do this for both the aix
host and aix build.
Sorry about the delay,
jeff
