[edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules when multiple output files
Bob Feng
bob.c.feng at intel.com
Tue Jun 16 08:48:51 UTC 2020
Hi Pierre,
Yes. It's fine for me.
Thanks,
Bob
-----Original Message-----
From: Pierre Gondois <Pierre.Gondois at arm.com>
Sent: Monday, June 15, 2020 11:58 PM
To: Feng, Bob C <bob.c.feng at intel.com>; devel at edk2.groups.io
Cc: Gao, Liming <liming.gao at intel.com>; Sami Mujawar <Sami.Mujawar at arm.com>; Tomas Pilar <Tomas.Pilar at arm.com>; nd <nd at arm.com>
Subject: RE: [edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules when multiple output files
Hello Bob,
I have locally corrected the patch. On non-arm architecture, I had previously tested it for the following configurations, but this wasn't enough apparently. I am currently testing it on more platforms and I will send you the list of the tested platforms along with the V2.
Plarform="Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc"|"Platform/Intel/BoardModulePkg/BoardModulePkg.dsc"
Host=Windows|Linux
BuildType=DEBUG|RELEASE
Toolchain=VS2017|GCC5
Arch=X64|IA32
Before submitting a V2, I am planning to create a pull request on the edk2 github repository, allowing me to check whether it passes the CI tests. Would it be fine for you?
About your comment about changing the logic of ApplyBuildRule, the logic of _ApplyBuildRule currently breaks out of the loop whenever a final target is found. The new rule on ".amli" files needs to continue looking for rules to apply, otherwise the processing on the initial ASL file stops when the AML file is generated (thus, not continuing the path ".amli"->".c"->".obj"). I agree this is hard to check, but I believe the edk2 CI tests should allow to put more trust in the patch serie.
Regards,
Pierre
-----Original Message-----
From: Feng, Bob C <bob.c.feng at intel.com>
Sent: 10 June 2020 04:58
To: Pierre Gondois <Pierre.Gondois at arm.com>; devel at edk2.groups.io
Cc: Gao, Liming <liming.gao at intel.com>; Sami Mujawar <Sami.Mujawar at arm.com>; Tomas Pilar <Tomas.Pilar at arm.com>; nd <nd at arm.com>
Subject: RE: [edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules when multiple output files
Hi Pierre,
I found there is an incorrect target generated in the OvmfPkg/AcpiTables Makefile when I tried to build Ovmf. That incorrect target causes ovmf to build failed.
$(DEBUG_DIR)\PlatformAcpiTables : $(MAKE_FILE) $(DEBUG_DIR)\PlatformAcpiTables : $(STATIC_LIBRARY_FILES) $(DEBUG_DIR)\PlatformAcpiTables : $(STATIC_LIBRARY_FILES_LIST)
"$(DLINK)" $(DLINK_FLAGS) $(DLINK_SPATH) @$(STATIC_LIBRARY_FILES_LIST)
Regarding this patch, I think it changes the logic of the _ApplyBuildRule, replacing "break" with "continue" and removing some if and elseif blocks, so it would be hard for me to make sure your current logic can cover all the original cases. Would you show me how many testing you have done?
Thanks,
Bob
-----Original Message-----
From: Pierre Gondois <Pierre.Gondois at arm.com>
Sent: Monday, June 8, 2020 10:01 PM
To: Feng, Bob C <bob.c.feng at intel.com>; devel at edk2.groups.io
Cc: Gao, Liming <liming.gao at intel.com>; Sami Mujawar <Sami.Mujawar at arm.com>; Tomas Pilar <Tomas.Pilar at arm.com>; nd <nd at arm.com>
Subject: RE: [edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules when multiple output files
Hello Bob,
Should I modify the patch ?
Regards,
Pierre
-----Original Message-----
From: Pierre Gondois
Sent: Tuesday, June 2, 2020 2:04 PM
To: Feng, Bob C <bob.c.feng at intel.com>; devel at edk2.groups.io
Cc: Gao, Liming <liming.gao at intel.com>; Sami Mujawar <Sami.Mujawar at arm.com>; Tomas Pilar <Tomas.Pilar at arm.com>; nd <nd at arm.com>
Subject: RE: [edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules when multiple output files
Hello Bob,
Thank you for your answer,
I put my comments as [Pierre],
Regards,
Pierre
-----Original Message-----
From: Feng, Bob C <bob.c.feng at intel.com>
Sent: 02 June 2020 12:16
To: devel at edk2.groups.io; Pierre Gondois <Pierre.Gondois at arm.com>
Cc: Gao, Liming <liming.gao at intel.com>; Sami Mujawar <Sami.Mujawar at arm.com>; Tomas Pilar <Tomas.Pilar at arm.com>; nd <nd at 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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of PierreGondois
Sent: Monday, May 18, 2020 10:11 PM
To: devel at edk2.groups.io
Cc: Pierre Gondois <Pierre.Gondois at arm.com>; Feng, Bob C <bob.c.feng at intel.com>; Gao, Liming <liming.gao at intel.com>; Sami.Mujawar at arm.com; Tomas.Pilar at arm.com; nd at arm.com
Subject: [edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules when multiple output files
From: Pierre Gondois <pierre.gondois at 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 at 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)'
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61320): https://edk2.groups.io/g/devel/message/61320
Mute This Topic: https://groups.io/mt/74291744/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list