Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix overlapping comparison warning #88

Conversation

edwarddavidbaker
Copy link

NumSamples can not be 8 and 16 at the same time. Closes #87.

warning: overlapping comparisons always evaluate to false [-Wtautological-overlap-compare]
Source/GmmLib/Texture/GmmGen9Texture.cpp:595:46:
           ((pTexInfo->MSAA.NumSamples == 8) && (pTexInfo->MSAA.NumSamples == 16)) &&
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

NumSamples can not be 8 and 16 at the same time. Closes intel#87.

warning: overlapping comparisons always evaluate to false [-Wtautological-overlap-compare]
Source/GmmLib/Texture/GmmGen9Texture.cpp:595:46:
           ((pTexInfo->MSAA.NumSamples == 8) && (pTexInfo->MSAA.NumSamples == 16)) &&
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@johnmach
Copy link
Contributor

johnmach commented Oct 6, 2021

Thanks Edward. Might have overwritten. Will update this in the repo.

@edwarddavidbaker
Copy link
Author

Thanks John! Fixed in d84606b .

@edwarddavidbaker edwarddavidbaker deleted the msaa-numsamples-warning-2 branch October 6, 2021 15:04
@paulmenzel
Copy link

@johnmach, out of curiosity, why didn’t you merge this merge/pull request, and instead authored the commit d84606b (Fix Overlapping Comparision)](#66) yourself, which also seems to reference the wrong issue (#66 instead of #87) and misses a commit message body, @edwarddavidbaker’s commit in this merge/pull request has?

@johnmach
Copy link
Contributor

@paulmenzel

Thanks for raising this.

  1. out of curiosity, why didn’t you merge this merge/pull request, and instead authored the commit d84606b (Fix Overlapping Comparision) yourself

Gmmlib follows a standard process of bringing in changes to opensource https://github.com/intel/gmmlib. Firstly, for any external PR/issues the fix is merged in internal repo and then after certain test cycles and validation it is promoted to open source. At the same time the corresponding external PR/issues is closed referencing the fix commit. I see in above closed PR, edwarddavidbaker closed this referencing to the fix commit d84606b.

  1. which also seems to reference the wrong issue (FindThreads.cmake:121 (CHECK_INCLUDE_FILE) #66 instead of GmmGen9Texture overlapping MSAA.NumSamples comparisons #87).

I agree. This is referencing the wrong issue. This is due to the internal commit pull request i.e. X appended to the commit title in internal repo. The same commit flows to the open source repo for which the github renders it has #X referencing to external repo space. Hence, points out to wrong reference. We look forward to resolve this.

  1. and misses a commit message body, @edwarddavidbaker’s commit in this merge/pull request has

We would really like to take this suggestion going forward. We will try to incorporate all commit details of external PR/issue in our internal merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GmmGen9Texture overlapping MSAA.NumSamples comparisons
3 participants