[PATCH] Verify source files for a module only once during the build

[Argyrios Kyrtzidis]

+def fmodules_skip_verify_if_validated_after : Joined<["-"], "fmodules-skip-verify-if-validated-after=">,

Should we use a more general name, like “-fbuild-session-timestamp” or something, this seems like it could have other uses in the future.

+static void updateModuleTimestamp(ModuleFile &MF) {
+  // Overwrite the timestamp file contents so that file's mtime changes.
+  std::string TimestampFilename = MF.getTimestampFilename();
+  std::string ErrorInfo;
+  llvm::raw_fd_ostream OS(TimestampFilename.c_str(), ErrorInfo);
+  if (!ErrorInfo.empty())
+    return;
+  OS << "Timestamp file\n";
+}

+    New->InputFilesValidationTimestamp = 0;
+    if (New->Kind == MK_Module) {
+      std::string TimestampFilename = New->getTimestampFilename();
+      llvm::sys::fs::file_status Status;
+      // A cached stat value would be fine as well.
+      if (!FileMgr.getNoncachedStatValue(TimestampFilename, Status)) {
+        llvm::sys::TimeValue MTime = Status.getLastModificationTime();
+        New->InputFilesValidationTimestamp =
+            MTime.toEpochTime() * 1000 + MTime.milliseconds();
+      }
+    }
+

I’m concerned that we are depending on being in-sync with the modification time given by the file system and the builder that provided the original timestamp.

I think we should only use the timestamp the builder is providing. For example:

— "-timestamp=123"
— No timestamp file
	- verify the module
	- once done, write out “123” to the timestamp file
— Timestamp file
	- read timestamp file and compare the timestamp given in command-line with the one written to the timestamp file.

What do you think ?


On Feb 7, 2014, at 10:18 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:

> On Fri, Feb 7, 2014 at 6:14 PM, Ben Langmuir <blangmuir at apple.com> wrote:
>> Hi Dmitri,
>> 
>> +        // If we are reading a module, we will create a verification timestamp,
>> +        // so we verify all input files.  Otherwise, verify only user input
>> +        // files.
>> +        unsigned LastFileToValidate =
>> +            F.Kind == MK_Module ? F.InputFilesLoaded.size() : Record[1];
>> +        for (unsigned I = 0; I < LastFileToValidate; ++I) {
>>            InputFile IF = getInputFile(F, I+1, Complain);
>>            if (!IF.getFile() || IF.isOutOfDate())
>> 
>> 
>> Should that be
>> 
>> MK_Module && SkipVerifyIfValidatedAfter ?
>> 
>> or is it intentional to stat the system files even if this new command line option is missing?
> 
> Indeed, that was not intended.  Thanks!  Updated and rebased patch attached.
> 
> Dmitri
> 
> -- 
> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
> <optimize-module-verification-v3.patch>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140207/dda30cc3/attachment.html>