[Bug]: CLANG build of ArmGicLib failed due to ICC_SGI1R declaration about mu_silicon_arm_tiano HOT 11 CLOSED

@gus33000 Could you please let me know if this #83 works for your scenario? We can merge it once you verified it works. Thanks.

from mu_silicon_arm_tiano.

Works fine here, (builds fine and boots fine) with the ICC_SGI1R_EL1 workaround I previously used removed. Thanks!

@kuqin12

Thanks a lot for your verification. I will merge the PR once people sign it off.

from mu_silicon_arm_tiano.

The change is merged. Closing this ticket. Will try to upstream this entire SGI function change to edk2 with the updated version shortly.

from mu_silicon_arm_tiano.

I didn't see a problem in local GCC builds. If this is only a problem on CLANG, that's expected since it is not tested in CI. Otherwise, we need to make sure this can be caught in CI as part of the fix.

from mu_silicon_arm_tiano.

This issue has been automatically marked as stale because it has not had activity in 45 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions.

from mu_silicon_arm_tiano.

This issue has been automatically been closed because it did not have any activity in 45 days and no follow up within 7 days after being marked stale. Thank you for your contributions.

from mu_silicon_arm_tiano.

This issue has been automatically marked as stale because it has not had activity in 45 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions.

from mu_silicon_arm_tiano.

Right now I'm still working around this issue by using a define in the command line arguments, but I assume it would be better here to not just correct the name used in the code but also introduce a clang ci?

Would be great if we could have newer clang versions supported in the process, I saw some work recently around getting CLANGPDB to work (would also help provide compiles under Windows itself for ARM64).

If it's just a define rename I can provide a PR fine for this. Would any other toolchain be impacted by such a rename? Does GCC support both names?

I was under the impression looking at the code above anyway gcc would get these redefined.

Slightly offtopic but there are other issues in general about some pip packages not working under Linux ARM64 either to compile here but that would be for an entirely different issue on its own.

from mu_silicon_arm_tiano.

Sorry for the delay. The support of CLANGPDB for ARM build has other issues at this point, which is why there is nothing as such yet. But we do want to support that properly in the future.

I do not think that renaming the definition will cause any issue, would you mind creating a PR? Otherwise, I will get to it later today.

On the other hand, I would expect that building on Linux ARM will come before the CLANGPDB support because this is more in our control.

from mu_silicon_arm_tiano.

Sure I will test this now

from mu_silicon_arm_tiano.

Works fine here, (builds fine and boots fine) with the ICC_SGI1R_EL1 workaround I previously used removed. Thanks!

@kuqin12

from mu_silicon_arm_tiano.