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 <[email protected]>
Sent: 10 June 2020 04:58
To: Pierre Gondois <[email protected]>; [email protected]
Cc: Gao, Liming <[email protected]>; Sami Mujawar <[email protected]>;
Tomas Pilar <[email protected]>; nd <[email protected]>
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 <[email protected]>
Sent: Monday, June 8, 2020 10:01 PM
To: Feng, Bob C <[email protected]>; [email protected]
Cc: Gao, Liming <[email protected]>; Sami Mujawar <[email protected]>;
Tomas Pilar <[email protected]>; nd <[email protected]>
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 <[email protected]>; [email protected]
Cc: Gao, Liming <[email protected]>; Sami Mujawar <[email protected]>;
Tomas Pilar <[email protected]>; nd <[email protected]>
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 <[email protected]>
Sent: 02 June 2020 12:16
To: [email protected]; Pierre Gondois <[email protected]>
Cc: Gao, Liming <[email protected]>; Sami Mujawar <[email protected]>;
Tomas Pilar <[email protected]>; nd <[email protected]>
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: [email protected] <[email protected]> On Behalf Of PierreGondois
Sent: Monday, May 18, 2020 10:11 PM
To: [email protected]
Cc: Pierre Gondois <[email protected]>; Feng, Bob C
<[email protected]>; Gao, Liming <[email protected]>;
[email protected]; [email protected]; [email protected]
Subject: [edk2-devel] [PATCH v1 1/3] BaseTools: Generate multiple rules when
multiple output files
From: Pierre Gondois <[email protected]>
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 <[email protected]>
---
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 (#61303): https://edk2.groups.io/g/devel/message/61303
Mute This Topic: https://groups.io/mt/74291744/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
