public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "PierreGondois" <pierre.gondois@arm.com>
To: "Feng, Bob C" <bob.c.feng@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Gao, Liming" <liming.gao@intel.com>,
	Sami Mujawar <Sami.Mujawar@arm.com>,
	Tomas Pilar <Tomas.Pilar@arm.com>, nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules when multiple output files
Date: Tue, 2 Jun 2020 13:03:53 +0000	[thread overview]
Message-ID: <DB7PR08MB311316BA366D3740D68B3D6F8B8B0@DB7PR08MB3113.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <BN6PR11MB0068BCC4E81A8762595AFBCAC98B0@BN6PR11MB0068.namprd11.prod.outlook.com>

Hello Bob,
Thank you for your answer,
I put my comments as [Pierre],

Regards,
Pierre


-----Original Message-----
From: Feng, Bob C <bob.c.feng@intel.com> 
Sent: 02 June 2020 12:16
To: devel@edk2.groups.io; Pierre Gondois <Pierre.Gondois@arm.com>
Cc: Gao, Liming <liming.gao@intel.com>; Sami Mujawar <Sami.Mujawar@arm.com>; Tomas Pilar <Tomas.Pilar@arm.com>; nd <nd@arm.com>
Subject: RE: [edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules when multiple output files

My comments are inline marked as [Bob].

Thanks,
Bob

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of PierreGondois
Sent: Monday, May 18, 2020 10:11 PM
To: devel@edk2.groups.io
Cc: Pierre Gondois <Pierre.Gondois@arm.com>; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>; Sami.Mujawar@arm.com; Tomas.Pilar@arm.com; nd@arm.com
Subject: [edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules when multiple output files

From: Pierre Gondois <pierre.gondois@arm.com>

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2425

This patch modifies the Makefile generation not to stop adding Makfile rules when the first final target is found.
E.g.:
If the following rules are described in build_rule.txt:
 -[Rule1]: .X files generate .Y and .Z files;
 -[Rule2]: .Z files generate .Z1 files.
Currently, if a File1.X file was part of the sources of a module, only [Rule1] would be generated in the Makefile.
Indeed, there are no rules to apply to .Y files: .Y files are a final target. However, there is still [Rule2] to apply to .Z files.

[Bob] I think currently a rule's output file will be added back to source file list, and in the later loop, that output file will be handled by another rule.  Doesn't that algorithm handle your case above?
[Pierre]
The rule's output file was effectively added to the list of source files, and a rule was searched  for this output file. However, the loop stopped when the first final target was found. By final target I mean "a file that isn't the input of a rule".
For the asl/aml/amli case, this meant that:
 -(first loop iteration: treating the ASL file) The rule for ASL files was found and applied to the input ASL file. The AML and ".amli" files were added to the list of source files to look for a rule to apply to them. The loop continues.
 -(second loop iteration: treating the AML file) There is no rule for AML files. This means that the AML file is a final target. The loop ends without having applied the rule on the ".amli" file.
This is why all the "break" instructions of the loop have been replaced with "continue". This prevents the loop from exiting without having treated all the files in the "SourceList".

This patch also adds a dependency between the first ouput file of a rule and the other output files.
For instance, with the same example as above, File1.Y and File1.Z are generated by the following rule:
File1.Y: File1.X
    <Generate File1.Y>
    <Generate File1.Z>

and the new dependency is:
File1.Z: File1.Y

This is necessary to keep a dependency order during the execution of the Makefile. Indeed, .Y and .Z files are generated by the execution of a common set of commands, and without this rule, there is no explicit dependency relation between them.

[Bob] If there are 3 output files, for example: 
<OutputFile>
        A
        B
        C
Will the dependency relationship be
B: A
C: B

[Pierre]
Currently, the dependency relation that would be created is:
B: A
C: A
This can be changed to the dependency you described.

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---

The changes can be seen at: https://github.com/PierreARM/edk2/tree/803_Compile_AML_bytecode_array_into_OBJ_file_v1

Notes:
    v1:
     - Generate multiple rules when multiple output files
       are specified in the build_rule.txt file. [Pierre]

 BaseTools/Source/Python/AutoGen/GenMake.py       |  6 +++
 BaseTools/Source/Python/AutoGen/ModuleAutoGen.py | 40 ++++++++++----------
 2 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py b/BaseTools/Source/Python/AutoGen/GenMake.py
index bbb3c29446f53fa7f2cb61a216a5b119f72c3fbc..0314d0ea34d99a014379e8d30c46ac0f0a7068ce 100755
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -1054,6 +1054,12 @@ cleanlib:
                     TargetDict = {"target": self.PlaceMacro(T.Target.Path, self.Macros), "cmd": "\n\t".join(T.Commands),"deps": Deps}
                     self.BuildTargetList.append(self._BUILD_TARGET_TEMPLATE.Replace(TargetDict))
 
+                    # Add a Makefile rule for targets generating multiple files.
+                    # The main output is a prerequisite for the other output files.
+                    for i in T.Outputs[1:]:
+                        AnnexeTargetDict = {"target": 
+ self.PlaceMacro(i.Path, self.Macros), "cmd": "", "deps": 
+ self.PlaceMacro(T.Target.Path, self.Macros)}
+
+ self.BuildTargetList.append(self._BUILD_TARGET_TEMPLATE.Replace(Annexe
+ TargetDict))
+
     def ParserCCodeFile(self, T, Type, CmdSumDict, CmdTargetDict, CmdCppDict, DependencyDict):
         if not CmdSumDict:
             for item in self._AutoGenObject.Targets[Type]:
diff --git a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
index aad591de65f086043d55aeea5661f59c53792e7c..82facdb2774c56833718317f4414e0c1861de47f 100755
--- a/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
@@ -860,7 +860,8 @@ class ModuleAutoGen(AutoGen):
         SubDirectory = os.path.join(self.OutputDir, File.SubDir)
         if not os.path.exists(SubDirectory):
             CreateDirectory(SubDirectory)
-        LastTarget = None
+        TargetList = set()
+        FinalTargetName = set()
         RuleChain = set()
         SourceList = [File]
         Index = 0
@@ -872,7 +873,7 @@ class ModuleAutoGen(AutoGen):
         while Index < len(SourceList):
             Source = SourceList[Index]
             Index = Index + 1
-
+            FileType = TAB_UNKNOWN_FILE
             if Source != File:
                 CreateDirectory(Source.Dir)
 
@@ -881,34 +882,27 @@ class ModuleAutoGen(AutoGen):
                 if not self.IsLibrary:
                     continue
                 RuleObject = self.BuildRules[TAB_DEFAULT_BINARY_FILE]
-            elif FileType in self.BuildRules:
-                RuleObject = self.BuildRules[FileType]
             elif Source.Ext in self.BuildRules:
                 RuleObject = self.BuildRules[Source.Ext]
             else:
-                # stop at no more rules
-                if LastTarget:
-                    self._FinalBuildTargetList.add(LastTarget)
-                break
-
+                # No more rule to apply: Source is a final target.
+                FinalTargetName.add(Source)
+                continue
             FileType = RuleObject.SourceFileType
             self._FileTypes[FileType].add(Source)
 
             # stop at STATIC_LIBRARY for library
             if self.IsLibrary and FileType == TAB_STATIC_LIBRARY:
-                if LastTarget:
-                    self._FinalBuildTargetList.add(LastTarget)
-                break
+                FinalTargetName.add(Source)
+                continue
 
             Target = RuleObject.Apply(Source, self.BuildRuleOrder)
             if not Target:
-                if LastTarget:
-                    self._FinalBuildTargetList.add(LastTarget)
-                break
-            elif not Target.Outputs:
-                # Only do build for target with outputs
-                self._FinalBuildTargetList.add(Target)
+                # No Target: Source is a final target.
+                FinalTargetName.add(Source)
+                continue
 
+            TargetList.add(Target)
             self._BuildTargets[FileType].add(Target)
 
             if not Source.IsBinary and Source == File:
@@ -916,12 +910,16 @@ class ModuleAutoGen(AutoGen):
 
             # to avoid cyclic rule
             if FileType in RuleChain:
-                break
+                EdkLogger.error("build", ERROR_STATEMENT, "Cyclic 
+ dependency detected while generating rule for %s" % str(Source))
 
             RuleChain.add(FileType)
             SourceList.extend(Target.Outputs)
-            LastTarget = Target
-            FileType = TAB_UNKNOWN_FILE
+
+        # For each final target, retrieve the TargetDescBlock instance.
+        for FTargetName in FinalTargetName:
+            for Target in TargetList:
+                if FTargetName == Target.Target:
+                    self._FinalBuildTargetList.add(Target)
 
     @cached_property
     def Targets(self):
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'





  reply	other threads:[~2020-06-02 13:04 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-18 14:11 [PATCH v1 0/3] Compile AML bytecode array into OBJ file PierreGondois
2020-05-18 14:11 ` [PATCH v1 1/3] BaseTools: Generate multiple rules when multiple output files PierreGondois
2020-06-02 11:15   ` [edk2-devel] " Bob Feng
2020-06-02 13:03     ` PierreGondois [this message]
2020-06-08 14:00       ` PierreGondois
2020-06-10  3:58         ` Bob Feng
2020-06-15 15:57           ` PierreGondois
2020-06-16  8:48             ` Bob Feng
2020-05-18 14:11 ` [PATCH v1 2/3] BaseTools: Rename AmlToHex script to AmlToC PierreGondois
2020-05-18 14:11 ` [PATCH v1 3/3] BaseTools: Compile AML bytecode arrays into .obj file PierreGondois
2020-05-18 14:31 ` [PATCH v1 0/3] Compile AML bytecode array into OBJ file PierreGondois
     [not found] ` <161025E2E32EBB17.23434@groups.io>
2020-05-22 12:01   ` [edk2-devel] " PierreGondois
     [not found]   ` <161157FAC70B55F8.21259@groups.io>
2020-05-28 11:03     ` PierreGondois
2020-05-28 11:10       ` Bob Feng
2020-06-02 11:15       ` Bob Feng
2020-06-13  9:37 ` Ard Biesheuvel
2020-06-15 16:08   ` PierreGondois

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DB7PR08MB311316BA366D3740D68B3D6F8B8B0@DB7PR08MB3113.eurprd08.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox