[llvm-profdata] Remove unnecessary file size check

[huangjd]

Summary

Unsure why profile reader checks profile size to be less than 4 GB. This breaks builds using a very large profile.
The limit is not seen anywhere else, so I am not sure why is it there in the first place.

[snehasish]

Some filesystems have a 4GB limit on file size (notably FAT32). Given that llvm-profdata as a tool is built for multiple platforms, I don't think we should remove this check.

[huangjd]

In D140741#4018895, @snehasish wrote:

Some filesystems have a 4GB limit on file size (notably FAT32). Given that llvm-profdata as a tool is built for multiple platforms, I don't think we should remove this check.

This check is in the Reader, not Writer. The entire file is already read at this point so if there's any IO error it would happen at line 1835, which is already checked.

[snehasish]

In D140741#4018926, @huangjd wrote:

In D140741#4018895, @snehasish wrote:

Some filesystems have a 4GB limit on file size (notably FAT32). Given that llvm-profdata as a tool is built for multiple platforms, I don't think we should remove this check.

This check is in the Reader, not Writer. The entire file is already read at this point so if there's any IO error it would happen at line 1835, which is already checked.

I see, looking at the commit which added the code doesn't seem to give any more clues apart from it being a sanity check. So then it might be ok to remove?
https://github.com/llvm/llvm-project/commit/c572e92c763f44bddfa30bc43e9027547b72dbfa

Not sure if @dnovillo is active.

[arsenm]

In D140741#4018895, @snehasish wrote:

Some filesystems have a 4GB limit on file size (notably FAT32). Given that llvm-profdata as a tool is built for multiple platforms, I don't think we should remove this check.

Let the file system check that, don't force artificial restrictions in random places

[huangjd]

The check also looks suspicious by casting size_t to uint64_t. If size_t is 64 bit the cast has no effect, and if size_t is 32 bit then the condition is always false, so I don't understand what's the purpose of this check

[davidxl]

The check looks unnecessary or misplaced.

llvm/lib/ProfileData/SampleProfReader.cpp

 /// \returns an error code indicating the status of the buffer.
 static ErrorOr<std::unique_ptr<MemoryBuffer>>
 setupMemoryBuffer(const Twine &Filename) {
   auto BufferOrErr = MemoryBuffer::getFileOrSTDIN(Filename, /*IsText=*/true);
   if (std::error_code EC = BufferOrErr.getError())
     return EC;
   auto Buffer = std::move(BufferOrErr.get());
 
-  // Check the file.
-  if (uint64_t(Buffer->getBufferSize()) > std::numeric_limits<uint32_t>::max())
-    return sampleprof_error::too_large;
-
   return std::move(Buffer);
 }
 
 /// Create a sample profile reader based on the format of the input file.
 ///
 /// \param Filename The file to open.
 ///
 /// \param C The LLVM context to use to emit diagnostics.